| Summary: | Wrong GLES2 pixel format chosen on Android | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Eluan Costa Miranda <eluancm> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | trivial | ||
| Priority: | P2 | CC: | gabomdq, philipp.wiesemann |
| Version: | HG 2.0 | Keywords: | target-2.0.0 |
| Hardware: | ARM | ||
| OS: | Android (All) | ||
| Attachments: | EGL config patch | ||
I have not tried the patch but here are some comments (they are unrelated to its usefulness, feel free to ignore them): Creating the "value" array could be moved out of the loop. Else (worst case) there would be 128 arrays created on the heap but actually one is enough because it could be reused. The inner loop could be left as soon as "accept" is set to "false" because it can not become true again and therefore continuing with the current config is a waste of nanoseconds. The line comment could be moved down because it comments something unrelated which might confuse some readers. Four spaces could be used for indentation like in the remaining file. The tabs from the patch assume a width of eight and therefore it will only look nice in editors with the same setting. I have not tried the patch but two comments its semantic changes: The patch changes that if no exactly matching config is found then no config is used at all and the program will not work. Currently it does but maybe with a "wrong" config. Therefore I think there should be no search for exactly matching config but for the best matching config. The values set with SDL_GL_SetAttribute() are minimum values. If for example a depth buffer of 16 is requested but the device only offers only 24 then the config would not be selected (with the algorithm from the patch). Shame upon me :-) It was written in five minutes and is obviously wrong, it was just to illustrate my point (and the IDE hides the identation,) sorry about that, it worked for the two test devices I have. I've only used it for a moment, now I've implemented another workaround for the unrelated issue of the Adreno 2xx drivers compositing the framebuffer alpha (leading to hall of mirror effect on engines like darkplaces-quake.) But if someone really needs another pixel format, SDL may still not return the right option. At least the report is valid :-) Can you update/cleanup this patch with Phillips suggestions? I'll make sure to test it and commit it. Thanks! (Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.) Tagging a bunch of bugs as target-2.0.0, Priority 2. This means we're in the final stretch for an official SDL 2.0.0 release! These are the bugs we really want to fix before shipping if humanly possible. That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.0 release, and generally be organized about what we're aiming to ship. Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment. Thanks! --ryan. This changeset should fix this issue: http://hg.libsdl.org/SDL/rev/b9ef4591d6b5 I tested this on my Nexus 4 which is returning one matching mode only, so feel free to re open if this explodes spectacularly! |
Created attachment 1126 [details] EGL config patch On android, when creating a GLES context, some drivers return the wrong configuration first, leading to a wrong pixel format because SDL2 always choses the first option. For example, if you ask for a RGB565 pixel format, the PowerVR SGX530 driver on the Motorola Defy returns RGBA8888 as the first available configuration and then SDL2 picks the wrong config. I've attached the quick hack I've put in my code, but it may not be up to your coding standards.