Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMove hit-test commands to be processed after document rendering. #2343
Conversation
|
r? @kvark |
|
So this fixes the flickering? Do we know why, other than "some assumptions were violated"? |
|
It seems to fix the flickering for me. I don't really know which assumptions were being violated, maybe kvark has a better idea? |
|
|
||
| let mut op = DocumentOps { | ||
| queries: vec![DocumentMsg::HitTest(pipeline_id, point, flags, tx)], | ||
| ..DocumentOps::nop() |
This comment has been minimized.
This comment has been minimized.
| let doc = self.documents.get_mut(&document_id).unwrap(); | ||
|
|
||
| match message { | ||
| DocumentMsg::HitTest(pipeline_id, point, flags, tx) => { |
This comment has been minimized.
This comment has been minimized.
kvark
Jan 26, 2018
Member
let's store the fields of this in a tuple instead of matching the enum again
| document_id: DocumentId, | ||
| message: DocumentMsg, | ||
| ) { | ||
| let doc = self.documents.get_mut(&document_id).unwrap(); |
This comment has been minimized.
This comment has been minimized.
kvark
Jan 26, 2018
Member
I think we'd end up with cleaner code if we fully inline this function (no need for separate function here)
I have an idea. // List of updates that need to be pushed to the
// gpu resource cache.
pub gpu_cache_updates: Option<GpuCacheUpdateList>,Applying these cache updates is basically synchronizing the RB and Renderer with regards to the GPU cache layout. Dropping those means the Renderer gets de-synchronized, and we end up using older/trashy GPU blocks for actual rendering. For some primitives, that results in geometry changes, for others - texture coordinates, properties, etc. |
|
Let's get this fix in (after a short cleanup), and I'll think about better ways to enforce the synchronization. |
This adds a new field to DocumentOp, which contains "queries" that update_document is meant to run after doing updates to the document. HitTest commands now just trigger a render (if needed) but defer the real work into a query. This ensures the hit-test is still run after the document is rendered, but is more in line with the structure of the current code, and does a better job of not violating implicit assumptions about document rendering/publishing. This fixes bustage from #2333.
|
Patch updated per review comments, let me know if I did it right. I wasn't quite sure I understood correctly what you meant when you said to use a tuple instead. Is it just for performance reasons? |
|
Thanks! |
No, just to avoid a redundant match. @bors-servo r+ |
|
|
Move hit-test commands to be processed after document rendering. This adds a new field to DocumentOp, which is a "query" that update_document is meant to run after doing updates to the document. HitTest commands now just trigger a render (if needed) but defer the real work into a query. This ensures the hit-test is still run after the document is rendered, but is more in line with the structure of the current code, and does a better job of not violating implicit assumptions about document rendering/publishing. This fixes bustage from #2333. <!-- 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/2343) <!-- Reviewable:end -->
|
|
Track GPU cache frame ID The PR separates frame rendering from GPU cache updates and enforces their order at run time. It's supposedly also fixing #2333, but in a different (complimentary!) way from #2343. cc @staktrace <!-- 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/2344) <!-- Reviewable:end -->
staktrace commentedJan 25, 2018
•
edited by larsbergstrom
This adds a new field to DocumentOp, which is a "query" that
update_document is meant to run after doing updates to the document.
HitTest commands now just trigger a render (if needed) but defer the
real work into a query. This ensures the hit-test is still run after the
document is rendered, but is more in line with the structure of the
current code, and does a better job of not violating implicit
assumptions about document rendering/publishing.
This fixes bustage from #2333.
This change is