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

Numeric precision in structs on mobile devices #3351

Open
gkjohnson opened this issue Oct 28, 2021 · 8 comments
Open

Numeric precision in structs on mobile devices #3351

gkjohnson opened this issue Oct 28, 2021 · 8 comments
Assignees

Comments

@gkjohnson
Copy link

gkjohnson commented Oct 28, 2021

Hello!

I've been doing some testing on my Android Pixel 3 device (Adreno 630 GPU) and I've noticed some oddities in the precision of numbers represented in structs. Specifically when running this fragment shader you can see that behavior is different when using an in as a struct member vs using an int:

// #define STRUCT_TEST

precision highp int;
struct TestStruct {
  highp int val;
};

void main() {
    
  #ifdef STRUCT_TEST
    
    // displays black
    TestStruct s;
    s.val = 1 << 20;
    gl_FragColor = vec4( s.val, s.val, s.val, 1.0 );

  #else

    // displays white
    int val = 1 << 20;
    gl_FragColor = vec4( val, val, val, 1.0 );

  #endif

}

I also put this test webpage together to try to understand the behavior I was dealing with. In it is a table that displays the apparent numeric precision of floats, ints, and uints derived from shifts bits in shaders until the value wraps to 0 and adding small values to floats until there is no value change. Here's what the table looks like on my Pixel 3. I'm seeing the same values on both Firefox and Chrome for Android. Others have reported a variety of other Pixel devices and others that report the same values:

As you can see the precision of the numbers in structs is half of what it is when using a regular variable.

It was recommended by @gfxprogrammer (Ken Russell) on twitter to share this information here to try to get to the bottom of the behavior. Specifically I'm interested in whether or not this is intended behavior / adhering to the WebGL spec? Is it something that can be fixed? Others have suggested that this maybe an intentional compatibility change by Qualcomm to support some specific apps. More concrete information would be appreciated if it's available.

Thank you!

@kdashg
Copy link
Contributor

kdashg commented Oct 28, 2021

It seems like it's dropping to mediump for that struct member. I believe this is a driver bug.

There is language in the spec that precision qualifiers only apply to their scope and within. Struct declarations do kinda look like local scopes, but clearly the intent is that they retain their precision. (FWIW there's no other way to change/adjust a struct or member's precision)

@lexaknyazev
Copy link
Member

Maybe related, two more Adreno integer bugs:

@gkjohnson
Copy link
Author

Thanks guys -- are there next steps here? Should a Chrome bug be made to inform the browser team? Is there a way to inform Qualcomm so this can maybe be fixed?

@Strepto
Copy link

Strepto commented Oct 20, 2022

This seems to still be an issue on a lot of android devices. Any progress on this?

Strepto added a commit to equinor/reveal that referenced this issue Oct 20, 2022
This works around an issue with struct precision on ardeno gpus: KhronosGroup/WebGL#3351
@kdashg
Copy link
Contributor

kdashg commented Oct 20, 2022

Someone should definitely inform Qualcomm if it's still an issue on new drivers for them.
It might be possible to do scalar decompositions of structs, but that's a pain.
We would benefit from a conformance test for this issue, in case it's different than #3190 and #3192.
Maybe that's why I'm assigned, but I don't remember.

@kenrussell
Copy link
Member

If the issue isn't resolved in Qualcomm's drivers, then the most feasible and general workaround would be to start using ANGLE's Vulkan backend to implement WebGL on these devices - it would be a drop-in replacement providing OpenGL ES and WebGL semantics. It'd be very difficult to transform WebGL shaders correctly in all situations to work around this driver bug.

@gkjohnson
Copy link
Author

Someone should definitely inform Qualcomm if it's still an issue on new drivers for them.

This is definitely still an issue on Android Pixel 3. I'm not aware of how to report anything like this to Qualcomm.

@Strepto
Copy link

Strepto commented Oct 21, 2022

Just to add a datapoint. The test webpage link from gkjohnson seems to give accurate values, and displays 10bit floating point mantissam and 16 bit int in fragment shader struct values on an Adreno 660, on an Asus Zenfone 8. Chome 106.0.5249.126, Android 12.

image

christjt pushed a commit to cognitedata/reveal that referenced this issue Oct 21, 2022
This works around an issue with struct precision on ardeno gpus: KhronosGroup/WebGL#3351
christjt pushed a commit to cognitedata/reveal that referenced this issue Nov 2, 2022
This works around an issue with struct precision on ardeno gpus: KhronosGroup/WebGL#3351
christjt added a commit to cognitedata/reveal that referenced this issue Nov 3, 2022
…ch (#2635)

* chore: add workflow that deploys to NPM on Github release (#2586)

* test: initial dry run publish

* chore: trigger release on released published

* fix: remove caching on release workflow

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* fix: expose and fix bug causing getBoundingBoxByNodeId/TreeIndex not to modify out parameter (#2472)

* refact: keyboard and mouse event handling in viewer (#2492)

* Intermediate commit

* Removed isFocused check in keyboard handling

* Keyboard active only which domElement is focused, mouse events are active when canvas is focused

* Refactored axisview eventlistener

* Reverted axisview event passed to domElement

* updated examples yarn.lock

* clean up

* leftover clean up

* Keyboard event listen on domElement, removed unnecessary event push to domElement from Axis cross

* removed unused variables

* clean up

* Fixed visual test error

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* fixed point cloud picking precision issue (#2508)

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* improvement: use 'high-performance' powerPreference to avoid non-discrete GPU being used on certain systems (#2512)

* feat: add metrics to MeasurementTool (#2523)

* chore: backport to old file structure

* fix: remove flat modifier, while maintaining TreeIndex precision (#2536)

Contribution by @Strepto (Equinor)

* fix: do not blend in-front and back frame buffers when back objects have not been rendered (#2540)

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* fix: visualization artifacts due to wrongfully handling cylinder clip planes (#2556)

* fix: simplify shader

* fix: set proper plane length

* fix: reduce some matrix transformations, and use built-in face-forward

* fix: plane magnitude not adjusting for scaled world transform

* fix: bad merge

* chore: revert CDF environments json

* chore: add primitive test fixture

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>

* improvement: make CadModelUpdateHandler trigger sector loading immediately after the camera stops moving (#2573)

* Make CadModelUpdateHandler trigger sector loading immediately after the camera stops moving

* Remove setting of redraw flag

* Update test

* Fix test

Co-authored-by: Christopher J. Tannum <[email protected]>

* Use vec2 instead of struct for packing TreeIndexes (#2585)

This works around an issue with struct precision on ardeno gpus: KhronosGroup/WebGL#3351

* fix: general cylinder inside rendering (#2620)

* fix: rendering if camera is inside primitive

* fix: reverse normal direction if hitting inside of cylinder

* fix: enable depth writing for in-front pass (#2621)

* Enable depth writing for in-front pass

* Set depth to minimum of in-front and back pass

Co-authored-by: Christopher J. Tannum <[email protected]>

* fix: remove unused pcMaterialManager from bad merge

* fix: re-create v8 test image due to cylinder clipping planes fix

Co-authored-by: cognite-bulldozer[bot] <51074376+cognite-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Lars Moastuen <[email protected]>
Co-authored-by: Pramod S <[email protected]>
Co-authored-by: Nils Henrik Hals <[email protected]>
Co-authored-by: eiriklegernaes <[email protected]>
Co-authored-by: Håkon Flatval <[email protected]>
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

No branches or pull requests

5 participants