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

[core] GL performance fixes #9255

Merged
merged 9 commits into from
Jun 14, 2017
Merged

Conversation

jfirebaugh
Copy link
Contributor

@jfirebaugh jfirebaugh commented Jun 13, 2017

Cherry-picks the following OpenGL performance and compatibility fixes to the release branch:

2017-06-13 12:17:58.318728-0700 Bench GL[7283:1811481] | paris      | 60.0 fps |
2017-06-13 12:17:58.318824-0700 Bench GL[7283:1811481] | paris2     | 60.0 fps |
2017-06-13 12:17:58.318910-0700 Bench GL[7283:1811481] | alps       | 60.2 fps |
2017-06-13 12:17:58.318992-0700 Bench GL[7283:1811481] | us east    | 60.1 fps |
2017-06-13 12:17:58.319074-0700 Bench GL[7283:1811481] | greater la | 60.1 fps |
2017-06-13 12:17:58.319212-0700 Bench GL[7283:1811481] | sf         | 60.1 fps |
2017-06-13 12:17:58.319564-0700 Bench GL[7283:1811481] | oakland    | 60.2 fps |
2017-06-13 12:17:58.319654-0700 Bench GL[7283:1811481] | germany    | 59.9 fps |
2017-06-13 12:17:58.319726-0700 Bench GL[7283:1811481] Total FPS: 480.6
2017-06-13 12:17:58.319797-0700 Bench GL[7283:1811481] Average FPS: 60.1

For #pragmas, don't generate varyings for attributes that aren't used by the fragment shader. Pack other varyings more tightly.
@kkaefer kkaefer added the Core The cross-platform C++ core, aka mbgl label Jun 13, 2017
We have an "oldBinding" value that we use for checking whether the vertex attribute was already
bound to the current VAO, but we never set the state. Additionally, we're also checking whether
the previous state was already any binding (optional is set), and don't re-enable the vertex
attribute array. Additionally, we now only disable the vertex attribute array when the previous
state was in fact an array attribute. We still don't deduplicate constant glVertexAttrib* calls,
but that's a little trickier.
@jfirebaugh
Copy link
Contributor Author

Don't review yet; I'm going to repurpose this PR to include the other GL performance fixes like #9185.

@jfirebaugh jfirebaugh changed the title [core] Reduce number of varyings to 8 or less [core] GL performance fixes Jun 13, 2017
@zugaldia
Copy link
Member

@jfirebaugh looks like Firebase was in a bad state, canceling the jobs and restarting them did the trick.

@jfirebaugh
Copy link
Contributor Author

Thanks @zugaldia. This is now ready for review. @kkaefer could you post benchmark results from the iPad mini?

Copy link
Contributor

@boundsj boundsj left a comment

Choose a reason for hiding this comment

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

Thanks @jfirebaugh @kkaefer. We can update the iOS release notes after this lands to note that there have been some performance fixes made under the hood.

@kkaefer
Copy link
Contributor

kkaefer commented Jun 14, 2017

0c275ad (current release branch tip)

name Run 1 Run 2 Run 3
paris 17.2 fps 17.2 fps 17.2 fps
paris2 19.4 fps 18.5 fps 19.3 fps
alps 8.8 fps 8.9 fps 8.8 fps
us east 24.4 fps 25.6 fps 25.0 fps
greater la 17.6 fps 18.0 fps 18.0 fps
sf 22.5 fps 21.7 fps 21.6 fps
oakland 18.9 fps 19.3 fps 19.0 fps
germany 10.1 fps 10.3 fps 10.1 fps

this PR

name Run 1 Run 2 Run 3
paris 47.6 fps 47.8 fps 47.6 fps
paris2 59.8 fps 59.9 fps 59.9 fps
alps 22.2 fps 22.2 fps 22.2 fps
us east 59.9 fps 59.8 fps 59.9 fps
greater la 47.4 fps 47.5 fps 47.5 fps
sf 60.1 fps 60.3 fps 60.3 fps
oakland 54.3 fps 54.2 fps 54.3 fps
germany 26.4 fps 26.4 fps 26.3 fps

@jfirebaugh jfirebaugh merged commit d277406 into release-ios-v3.6.0-android-v5.1.0 Jun 14, 2017
@jfirebaugh jfirebaugh deleted the cp-9251 branch June 14, 2017 18:13
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.

4 participants