Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EGL pixel format selection and OpenGL version parsing fixes #2438

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

binary1248
Copy link
Member

We've seen that in certain situations e.g. running on certain Android platforms (#2395) or running tests on the GitHub actions VMs, the OpenGL implementation can become pretty broken and start returning values that don't follow the specification. Because failing to parse an OpenGL context version doesn't really hinder us from continuing operation if everything else works, this change just assumes a conservative version and continues instead of causing the application to terminate.

The EGL pixel format selection procedure before this change relied on eglChooseConfig(). Using this function to select a pixel format bears the same problems as its wgl and glX counterparts (which we don't use): The way the SFML API is designed, the user has to resort to guessing a conservative configuration to request or else it will fail and graphics becomes unusable or even causes the application to terminate (see #2395). eglChooseConfig() takes a set of attributes that it tries to match exactly to a pixel format available on the system. If an exact match isn't found it will just fail. The other context implementations are a bit more "user friendly" in this respect, if an exact match isn't found they will still return something that is a close enough match and emit a warning to the console if a certain attribute could not be met. This gives the user helpful feedback and allows them to adjust their requested settings based on e.g. tester feedback. This changeset makes EGL pixel format selection follow the same procedure as the other context types.

Fixes #2395
Closes #2422

@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #2438 (ea7cd83) into master (7004db1) will increase coverage by 0.00%.
The diff coverage is 72.22%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2438   +/-   ##
=======================================
  Coverage   25.84%   25.84%           
=======================================
  Files         226      226           
  Lines       19419    19424    +5     
  Branches     4714     4715    +1     
=======================================
+ Hits         5018     5020    +2     
+ Misses      13868    13850   -18     
- Partials      533      554   +21     
Impacted Files Coverage Δ
src/SFML/Graphics/RenderTextureImplFBO.cpp 13.19% <0.00%> (-0.10%) ⬇️
src/SFML/Window/GlContext.cpp 72.67% <92.85%> (-0.06%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7004db1...ea7cd83. Read the comment docs.

@ChrisThrasher ChrisThrasher added this to Backlog in SFML 3.0.0 via automation Mar 7, 2023
@ChrisThrasher ChrisThrasher added this to the 3.0 milestone Mar 7, 2023
@eXpl0it3r eXpl0it3r moved this from Backlog to Clean Up Changes in SFML 3.0.0 Mar 7, 2023
@Zombieschannel
Copy link

So, I tried it out and my simple sfml green circle app crashed at the start with the error java.lang.UnsatisfiedLinkError: Unable to load native library "/data/app/~~w7gIWgbUbMdhkoSRfssnfw==/com.Zombieschannel.Tests-SZznDO1KI2CuOajkohUFkA==/lib/arm64/libsfml-activity-d.so": undefined symbol: ANativeActivity_onCreate
Unfortunately, after trying the latest code from the master branch, I seem to be getting the same crash, so either I'm doing something wrong or sfml 3 just does not work on android yet.
Also, I am just slightly confused as to why is this set as sfml 3.0 milestone when it is also an issue in sfml 2.

Copy link

@Zombieschannel Zombieschannel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, now that sfml 3 actually works on android, I tried this and the results are... questionable.
In normal sfml 3 I get these warnings/errors when running the app:
image

With these changes, I get the following:
image

But yeah generally, it does remove the OpenGL 0.0 error, but there still are errors.

My code for testing

#include <SFML/Graphics.hpp>
using namespace sf;

int main()
{
    sf::RenderWindow window;
    window.create(sf::VideoMode::getDesktopMode(), "SFML works!", sf::Style::Fullscreen);
    window.setFramerateLimit(10);
    sf::CircleShape shape(400.f);
    shape.setFillColor(sf::Color::Green);
    while (window.isOpen())
    {
        sf::Event event;
        while (window.pollEvent(event)) {
            if (event.type == sf::Event::Closed)
                window.close();
            if (event.type == sf::Event::LostFocus)
                window.setActive(0);
            if (event.type == sf::Event::GainedFocus)
                window.setActive(1);
            if (event.type == sf::Event::KeyReleased)
                if (event.key.code == sf::Keyboard::Escape)
                    window.close();
        }
        if (!window.hasFocus())
            continue;
        window.clear(Color::Black);
        window.draw(shape);
        window.display();
    }
}

@binary1248
Copy link
Member Author

That is the expected output with these changes. Because we are assuming version 1.1 if the version query fails, there is nothing for the settings check to warn about.

The context is still not being able to be set active, thus the error messages. Are you running the code in debug configuration? If you run the code in debug configuration, eglCheck should print additional information to the console explaining what is going wrong when trying to set the contexts active.

@binary1248
Copy link
Member Author

Rebased to master.

@binary1248
Copy link
Member Author

Rebased to master.

…r context types and make OpenGL context version parsing more tolerant of garbage data. Fixes #2395
@ChrisThrasher ChrisThrasher merged commit 6a3feda into master Apr 4, 2023
122 checks passed
@ChrisThrasher ChrisThrasher deleted the fix-opengl-es branch April 4, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
SFML 3.0.0
  
Clean Up Changes
Development

Successfully merging this pull request may close these issues.

Android OpenGL context 0.0 and antialiasing
5 participants