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

Don't require OpenGL 3.0 #6411

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Sep 12, 2024

Closes #6410

Why is this the best possible solution? Were any other approaches considered?

As I said in the comment:

/**
 * Checks if the device supports the given OpenGL ES version.
 *
 * Note: This approach may not be 100% reliable because `reqGlEsVersion` indicates
 * the highest version of OpenGL ES that the device's hardware is guaranteed to support
 * at runtime. However, it might not always reflect the actual version available.
 *
 * For a more reliable method, refer to https://developer.android.com/develop/ui/views/graphics/opengl/about-opengl#version-check.
 * This recommended approach is more complex to implement but offers better accuracy.
 */

The way I check OpenGL versions might not be the best one but for now, our main goal is to check if:

   <uses-feature
        android:glEsVersion="0x00030000"
        android:required="false" />

will let us make OpenGL 3.0 not required. If the way of checking versions is not reliable we will see crashes or receive reports from users and then we can fix it.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

@dbemke @srujner @WKobus do we have any device with OpenGL 2.0? You can check it using adb:
adb shell dumpsys SurfaceFlinger | grep "OpenGL

if not we can just check that using Mapbox is still available in settings as it was before.

Do we need any specific form for testing your changes? If so, please attach one.

No.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • added any new strings with date formatting to DateFormatsTest
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 marked this pull request as ready for review September 12, 2024 14:31
Copy link
Member

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

I verified that the resulting APK's manifest has OpenGL 3.0 marked as optional. I thought I could do this in Android Studio but I couldn't find a way to actually see the built manifest's content.

I used https://sisik.eu/apk-tool and saw the following:

	<uses-feature
		android:glEsVersion="20000"
		android:required="true"/>

	<uses-feature
		android:glEsVersion="30000"
		android:required="false"/>

I think it's fine to require openGL 2. All devices in the Play Store device library support it. We can file a separate issue to remove the runtime checks for Google Maps.

*
* Note: This approach may not be 100% reliable because `reqGlEsVersion` indicates
* the highest version of OpenGL ES that the device's hardware is guaranteed to support
* at runtime. However, it might not always reflect the actual version available.
Copy link
Member

Choose a reason for hiding this comment

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

Where did you get the info that it's not reliable?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not directly said anywhere but:

  1. If it was reliable it would probably be mentioned at https://developer.android.com/develop/ui/views/graphics/opengl/about-opengl#version-check. Instead we have that much more complex approach there.
  2. If you navigate to the declaration of reqGlEsVersion from ConfigurationInfo it says:
    The GLES version used by an application. The upper order 16 bits represent the major version and the lower order 16 bits the minor version.
    To me used by an application sounds like a version required programmatically or in the manifest file.
  3. On Stack Overflow I saw comments from other people claiming that it might not be reliable for example here https://stackoverflow.com/questions/25584032/is-there-a-way-to-check-if-android-device-supports-opengl-es-3-0 but also in other topics.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. What that does is check whether the OS version supports a certain openGL version but doesn't say anything about the specific device. So it's not very helpful for what we're trying to do but let's just leave it for now unless we see crashes reported.

@lognaturel lognaturel merged commit 4dbe61e into getodk:master Sep 12, 2024
6 checks passed
@lognaturel
Copy link
Member

I got tricked! I used the self-signed release build from CircleCI to check whether opengl 3 was successfully made optional. But that build excludes Mapbox so it's different than ones we do for release. The release builds that include Mapbox have opengl 3 required.

I added tools:replace="required" to the manifest uses-feature. Then I did see that opengl 3 had required false in the APK I built. It still didn't seem to work in the Play Store, though.

I think we should at least add tools:replace="required" and maybe @grzesiek2010 you'll have other ideas.

@dbemke
Copy link

dbemke commented Sep 13, 2024

do we have any device with OpenGL 2.0?

I checked 3 devices - all 3.2

@WKobus
Copy link

WKobus commented Sep 13, 2024

do we have any device with OpenGL 2.0?

I have OpenGL 3.2 on all my devices

@srujner
Copy link

srujner commented Sep 13, 2024

Same with me, 3.2 everywhere

@grzesiek2010
Copy link
Member Author

I think we should at least add tools:replace="required" and maybe @grzesiek2010 you'll have other ideas.

Hmm let's try to remove it from the manifest at all then #6414

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't require OpenGL 3.0
5 participants