Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Re-bind uniform locations after re-linking program #11583

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

ivovandongen
Copy link
Contributor

On some gl implementations the uniform locations are shifted after linking the program a second time, resulting in errors when using any uniform. (On some implementations they are actually doubled). Re-binding the uniforms after selectively binding the vertex attributes prevents this.

This currently prevents running on the Android emulator with hardware acceleration.

Fixes #11266

- On some gl implementations the uniform locations are shifted after linking the program a second time, resulting in errors when using any uniform. On some implementations they are actually doubled. Re-binding the uniforms after selectively binding the vertex attributes prevents this.
@ivovandongen ivovandongen added the Core The cross-platform C++ core, aka mbgl label Apr 3, 2018
@ivovandongen ivovandongen self-assigned this Apr 3, 2018
@ivovandongen ivovandongen requested a review from lbud April 3, 2018 13:31

// We have to re-initialize the uniforms state from the bindings as the uniform locations
// get shifted on some implementations
uniformsState = Uniforms::bindLocations(program);
Copy link
Member

Choose a reason for hiding this comment

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

Nice find!

@ivovandongen ivovandongen merged commit 4a0e025 into master Apr 5, 2018
@ivovandongen ivovandongen deleted the ivd-fix-android-emulator branch April 5, 2018 07:22
astojilj added a commit that referenced this pull request Apr 18, 2019
Remove re-linking programs as redundant.
It costs (cheaper to link once than twice) and is not that common GL API
usage pattern (subjective).
Initial idea was to enable work on optimization what would reduce number
of attrib setup calls in case that VAO is not available (#9433).

As such optimization is not implemented, we can remove re-linking.

Related to closed PR #9433 and PR #11583.
astojilj added a commit that referenced this pull request May 20, 2019
Remove re-linking programs as redundant.
It costs (cheaper to link once than twice) and is not that common GL API
usage pattern (subjective).
Initial idea was to enable work on optimization what would reduce number
of attrib setup calls in case that VAO is not available (#9433).

As such optimization is not implemented, we can remove re-linking.

Related to closed PR #9433 and PR #11583.
astojilj added a commit that referenced this pull request May 20, 2019
Remove re-linking programs as redundant.
It costs (cheaper to link once than twice) and (subjective) is not that common GL API
usage pattern, although perfectly legal and permitted.
Initial idea, behind the removed code, was to enable work on optimization
that would reduce number of attrib setup calls in case when VAO is not
available (as described in #9433).

As such optimization is not implemented, and it is arguable if it makes sense
to do it now, we can remove re-linking.

Related to closed PRs #9433 and PR #11583.

I have [measured the time spent just on relinking](https://gist.github.com/astojilj/29bd5a5c5dc0b2d9f29ecb660da07fbf) using release build on iPhone SE (A9, same as iPhone 6S):

- 1st run after reboot or installation
Total 37.14ms, average per program:1.86ms

- reopening
Total: 2.47ms, average per program: 0.12ms

This time we save using the patch here.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants