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

Batch similar composite operations together. #90

Merged
merged 1 commit into from Dec 2, 2015

Conversation

@pcwalton
Copy link
Collaborator

pcwalton commented Nov 21, 2015

Known bugs and limitations:

  • Only 32 composite operations can be batched, because that is the
    maximum size of the matrix palette.
  • Nested composite operations aren't correctly handled at the moment.
    They use the wrong device pixel ratio and read from and write to the
    same texture, which is undefined behavior (although usually works).
  • When a large number (>700 in my tests) of composite operations occur,
    there can be stray triangles drawn. I haven't been able to pin down
    why that occurs yet.

Please review this carefully if you get time; this is tricky stuff and I'm sure I've messed up :)

@glennw
Copy link
Member

glennw commented Nov 23, 2015

Unfortunately this breaks a lot of the ref tests (haven't checked against wpt/css yet). You can run ./mach test-ref --kind=wr --release to see the test failures.

@glennw
Copy link
Member

glennw commented Nov 23, 2015

I just realised that I forgot to copy across the new shaders - but even after doing that I still see a lot of the ref tests fail, and I think I have updated them correctly this time. When those are fixed I'll get it merged as soon as possible.

@pcwalton
Copy link
Collaborator Author

pcwalton commented Dec 1, 2015

Rebased. Tests not fixed yet.

@glennw
Copy link
Member

glennw commented Dec 1, 2015

@pcwalton I just found out that the mix-blend-mode shaders were broken (happened here glennw@5e4493d).

I've pushed an update of the shaders to servo + WR.

It's possible that this was the main issue with the reftests + composite batching - you might want to try the reftests just rebased on top of master and see if they just work now :)

@pcwalton pcwalton force-pushed the pcwalton:render-target-batching branch from d055e9a to 031dd08 Dec 2, 2015
@pcwalton
Copy link
Collaborator Author

pcwalton commented Dec 2, 2015

Updated to fix the issues I could see.

@@ -133,6 +139,18 @@ impl<'a> BatchBuilder<'a> {
primitive: Primitive,
vertices: &mut [PackedVertex],
tile_params: Option<TileParams>) {
// FIXME(pcwalton): Awful hack to work around a panic.
if self.vertex_buffer.vertices.len() + vertices.len() > u16::MAX as usize {

This comment has been minimized.

@glennw

glennw Dec 2, 2015

Member

Have you got any info on how to repro this? I would be keen to debug this and find the root cause if we have a repro (doesn't need to stop this merging though).

@pcwalton pcwalton force-pushed the pcwalton:render-target-batching branch from 031dd08 to a6bf439 Dec 2, 2015
@pcwalton
Copy link
Collaborator Author

pcwalton commented Dec 2, 2015

@glennw Removed the hack.

Known bugs and limitations:

* Only 32 composite operations can be batched, because that is the
  maximum size of the matrix palette.

* Nested composite operations aren't correctly handled at the moment.
  They use the wrong device pixel ratio and read from and write to the
  same texture, which is undefined behavior (although usually works).

* When a large number (>700 in my tests) of composite operations occur,
  there can be stray triangles drawn. I haven't been able to pin down
  why that occurs yet.
glennw added a commit that referenced this pull request Dec 2, 2015
Batch similar composite operations together.
@glennw glennw merged commit 95ea0c7 into servo:master Dec 2, 2015
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

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