Skip to content

/ webrender

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

Jump to bottom

Remove UBO usage. #457

Closed
glennw opened this issue Oct 21, 2016 · 2 comments
Closed

Remove UBO usage. #457

glennw opened this issue Oct 21, 2016 · 2 comments
Labels
difficulty: hard help wanted

Comments

@glennw
Copy link
Member

@glennw glennw commented Oct 21, 2016

Instead of using UBOs for submitting primitive instances and using gl_InstanceID, with the recent changes to prim_store, it would be better now to submit the PrimitiveInstance structures as instanced vertex attributes.
@glennw
Copy link
Member Author

@glennw glennw commented Nov 7, 2016

This may not be worth doing - it seems that instanced vertex attributes are only available in GL3.3+, and we want to support GL 3.1 for some older Intel GPU systems. If that's correct, then we can close this and stick with using UBOs. @jrmuizel or @kvark does that sound correct to you?
@kvark
Copy link
Member

@kvark kvark commented Nov 7, 2016

@glennw I disagree.

I don't think ARB_instanced_arrays support requirement limits us in any way. On Intel, 5th gen GPUs only support GL 2.1 (according to https://en.wikipedia.org/wiki/List_of_Intel_graphics_processing_units), while 6th gen (Sandy Bridge, starting 2011) already fits our needs:

3.1 on Windows
3.3 on OS
3.3 on Linux

On Windows we are using Direct3D anyway, which has InstanceDataStepRate field as of version 10. And on Linux/OSX we got GL 3.3 core support.

In general, if we are talking about GL extensions and not core GL versions, I haven't seen a system that would support instanced draw calls but not instanced arrays.

As for GLES, version 3.0 has our stuff included.
@kvark kvark mentioned this issue Nov 30, 2016
Merged
bors-servo added a commit that referenced this issue Dec 1, 2016
@bors-servo
Auto merge of #610 - kvark:instance, r=glennw
Loading status checks…
2bbb73e 
Vertex attributes cleanup

This is the preparation work for #457

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/610)
<!-- Reviewable:end -->
@kvark kvark mentioned this issue Dec 2, 2016
Merged
bors-servo added a commit that referenced this issue Dec 13, 2016
@bors-servo
Auto merge of #615 - kvark:instance, r=glennw
Loading status checks…
482b59b 
Instanced attributes

Closes #457

Performance-wise, I wasn't able to register a noticeable difference. Tested on https://github.com/servo/servo, full screen after the second page down, on `Mesa DRI Intel(R) HD Graphics 5500 (Broadwell GT2)` with resolution 2560x1440.

With the change, I got 4.5 ms mean GPU time in the first test, and 4.3 ms time in the second.
Without the change, I got 4.4 ms mean GPU time in the only test.

I suppose the difference is not visible since we are far from being VS-bound.

Note: this does not replace the optimization of having one large buffer in #456. Instanced attributes will benefit from it in the same way as UBOs.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/615)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
difficulty: hard help wanted
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@kvark @glennw
You can’t perform that action at this time.