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

Use texture layers for render target allocations, remove render phases. #478

Merged
merged 1 commit into from Oct 26, 2016

Conversation

@glennw
Copy link
Member

glennw commented Oct 25, 2016

Instead of splitting the scene into multiple phases, use texture
layers to allocate slices as needed for each pass of the render
frame. These are pooled and only allocated when the frame render
target configuration changes.

Previously, the code asserted if it was unable to fit the render
tasks into a single render target for a single tile. Now, this
(pathological) case is handled correctly.

This makes the upcoming dynamic primitive cache support a lot
simpler to add. It also means that we don't use a ping-pong
allocation style, which typically performs badly on mobile / tiled
rendering architectures.


This change is Reviewable

@glennw
Copy link
Member Author

glennw commented Oct 25, 2016

@glennw
Copy link
Member Author

glennw commented Oct 25, 2016

@pcwalton btw I tested this on mac mini hardware, since I know we've had troubles with driver stalls around texture / fbo allocs before. It seemed to be working nicely on my mac mini (and also on linux + osmesa).

vec2 st1 = vec2(src.screen_origin_task_origin.zw + src.size.xy) / 2048.0;
vUv = mix(st0, st1, aPosition.xy);
vec2 texture_size = vec2(textureSize(sCache, 0));
vec2 st0 = vec2(src.screen_origin_task_origin.zw) / texture_size;

This comment has been minimized.

@pcwalton

pcwalton Oct 25, 2016

Collaborator

nit: Do you need the vec2() cast here and below?

vUv0 = mix(st0, st1, aPosition.xy);
vec2 texture_size = vec2(textureSize(sCache, 0));
vec2 st0 = vec2(src0.screen_origin_task_origin.zw) / texture_size;
vec2 st1 = vec2(src0.screen_origin_task_origin.zw + src0.size_target_index.xy) / texture_size;

This comment has been minimized.

@pcwalton

pcwalton Oct 25, 2016

Collaborator

Same comment about vec2() as above

@pcwalton
Copy link
Collaborator

pcwalton commented Oct 25, 2016

r=me with a couple of nits

@bors-servo
Copy link
Contributor

bors-servo commented Oct 26, 2016

The latest upstream changes (presumably #445) made this pull request unmergeable. Please resolve the merge conflicts.

@glennw glennw force-pushed the glennw:texture-layers branch from fef4193 to 3751007 Oct 26, 2016
Instead of splitting the scene into multiple phases, use texture
layers to allocate slices as needed for each pass of the render
frame. These are pooled and only allocated when the frame render
target configuration changes.

Previously, the code asserted if it was unable to fit the render
tasks into a single render target for a single tile. Now, this
(pathological) case is handled correctly.

This makes the upcoming dynamic primitive cache support a lot
simpler to add. It also means that we don't use a ping-pong
allocation style, which typically performs badly on mobile / tiled
rendering architectures.
@glennw glennw force-pushed the glennw:texture-layers branch from 3751007 to 8928181 Oct 26, 2016
@pcwalton
Copy link
Collaborator

pcwalton commented Oct 26, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 26, 2016

📌 Commit 8928181 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Oct 26, 2016

Testing commit 8928181 with merge dcdc4d8...

bors-servo added a commit that referenced this pull request Oct 26, 2016
Use texture layers for render target allocations, remove render phases.

Instead of splitting the scene into multiple phases, use texture
layers to allocate slices as needed for each pass of the render
frame. These are pooled and only allocated when the frame render
target configuration changes.

Previously, the code asserted if it was unable to fit the render
tasks into a single render target for a single tile. Now, this
(pathological) case is handled correctly.

This makes the upcoming dynamic primitive cache support a lot
simpler to add. It also means that we don't use a ping-pong
allocation style, which typically performs badly on mobile / tiled
rendering architectures.

<!-- 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/478)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 26, 2016

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit 8928181 into servo:master Oct 26, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@glennw glennw deleted the glennw:texture-layers branch Oct 26, 2016
let (internal_format, gl_format) = gl_texture_formats_for_image_format(texture.format);
let type_ = gl_type_for_texture_format(texture.format);

gl::tex_image_3d(texture_id.target,

This comment has been minimized.

@kvark

kvark Oct 26, 2016

Member

This is concerning. I don't see the previous implementation actually reallocating the texture, like this one. Neither does the method name imply that the texture is going to be changed (create_fbo_for_texture_if_necessary).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.