Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@bdero
Copy link
Member

@bdero bdero commented Jun 13, 2023

No description provided.

@bdero bdero self-assigned this Jun 13, 2023
// `mediump` by default and provide macros for 16 bit types to keep writing
// cross API shaders easy.

#ifdef IMPELLER_TARGET_OPENGLES
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will work in general, the 16bit extensions aren't available everywhere and we can't actually support a backend that uses GLES and has 16bit uniform values, as the bindings will be incorrect.

Copy link
Member Author

Choose a reason for hiding this comment

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

16bit uniform buffer access is part of Vulkan 1.1 core, so this work on all of our targets. However, there are a few subproblems that I don't have time to work through right now: flutter/flutter#128836

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL.

we use the same generated headers on GLES and Vulkan though, so we'd need to solve that issue too - probably generate a different set of bindings assuming 32bit and 16bit float and select at runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

we use the same generated headers on GLES and Vulkan though

Just for Android, right? On MacOS, I think we end up using the Metal headers for all of the backends, which maybe is another problem that needs to be solved. :/

I'm still not really sure what the best design approach would be for reconciling the different backend headers. Maybe we can automate the common case and create a generic lowest common denominator meta-header with special UBO/VBO builders to dispatch the specific packing behavior of the backend in use. But importantly, using this is optional, and we can still use the per-backend struct directly for special backend-specific optimizations whenever there's a need.

Copy link
Member Author

@bdero bdero Jun 14, 2023

Choose a reason for hiding this comment

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

Actually, we can probably further avoid this problem by just finishing our implementation of 16 bit uniform access for the other backends. Even if we can't use 16 bit uniforms on GLES 2 devices, we still just enumerate and pull each uniform out of the buffer on the CPU, and so we can emulate it.

So if we invest some time in fixing the GLES and Vulkan incompatibilities, we might be able to continue getting away with sharing headers for multiple backends. (FYI @chinmaygarde)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added another issue for GLES here: flutter/flutter#128881

@bdero bdero force-pushed the bdero/vk-shader-floats branch 2 times, most recently from 17c4031 to e9ded53 Compare June 13, 2023 21:12
@bdero
Copy link
Member Author

bdero commented Jun 14, 2023

I have give up on solving the 16 bit buffer access problems for the time being, so closing this out.

Continuation issues:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants