{"action":"created","issue":{"url":"https://api.github.com/repos/servo/webrender/issues/3155","repository_url":"https://api.github.com/repos/servo/webrender","labels_url":"https://api.github.com/repos/servo/webrender/issues/3155/labels{/name}","comments_url":"https://api.github.com/repos/servo/webrender/issues/3155/comments","events_url":"https://api.github.com/repos/servo/webrender/issues/3155/events","html_url":"https://github.com/servo/webrender/issues/3155","id":366041530,"node_id":"MDU6SXNzdWUzNjYwNDE1MzA=","number":3155,"title":"dirty rect handling confusion?","user":{"login":"Gankro","id":1136864,"node_id":"MDQ6VXNlcjExMzY4NjQ=","avatar_url":"https://avatars1.githubusercontent.com/u/1136864?v=4","gravatar_id":"","url":"https://api.github.com/users/Gankro","html_url":"https://github.com/Gankro","followers_url":"https://api.github.com/users/Gankro/followers","following_url":"https://api.github.com/users/Gankro/following{/other_user}","gists_url":"https://api.github.com/users/Gankro/gists{/gist_id}","starred_url":"https://api.github.com/users/Gankro/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Gankro/subscriptions","organizations_url":"https://api.github.com/users/Gankro/orgs","repos_url":"https://api.github.com/users/Gankro/repos","events_url":"https://api.github.com/users/Gankro/events{/privacy}","received_events_url":"https://api.github.com/users/Gankro/received_events","type":"User","site_admin":false},"labels":[{"id":971349469,"node_id":"MDU6TGFiZWw5NzEzNDk0Njk=","url":"https://api.github.com/repos/servo/webrender/labels/priority:%20now","name":"priority: now","color":"a8bf00","default":false},{"id":263326148,"node_id":"MDU6TGFiZWwyNjMzMjYxNDg=","url":"https://api.github.com/repos/servo/webrender/labels/type:%20question","name":"type: question","color":"cc317c","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-10-02T19:16:28Z","updated_at":"2018-10-02T22:36:15Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"I'm walking through the dirty rect code and getting very confused, as we don't seem to be handling things in any kind of consistent manner.\r\n\r\nTwo cases I've noticed so far:\r\n\r\n\r\n------------\r\n\r\n`merge_dirty_rect` is non-commutative and very confusing:\r\n\r\nhttps://github.com/servo/webrender/blob/master/webrender/src/resource_cache.rs#L203\r\n\r\nWhy isn't this line `(_, None) => descriptor.full_rect()`, like the one below it? It seems to be inconsistent with the comment above, stating \"Update with no dirty rect. We want to reupload everything.\"\r\n\r\n\r\n------------\r\n\r\ndirty rect handling seems inconsistent in create_blob_scene_builder_requests\r\n\r\nhttps://searchfox.org/mozilla-central/source/gfx/webrender/src/resource_cache.rs#1144-1149,1166\r\n\r\nThe first section states that passing None into the blob renderer requests a *full* reupload, which makes sense and is consistent of my reading of Moz2dImageRenderer. However this code is grabbing the dirty rect off the template *and then subsequently sets the template rect to None*, indicating that after completing rendering, the whole image is now completely dirty? I think this should be setting the template's dirty_rect to `Some(rect::zero())`?\r\n\r\n------------\r\n\r\nI'm looking at changing all the dirty_rect code to using the following enum to make everything more clear, but I need what's happening in these cases to be clarified, in case I'm misunderstanding it.\r\n\r\n```rust\r\npub enum DirtyRect {\r\n    /// Nothing is Dirty, equivalent to SomeDirty(empty_rect)\r\n    NothingDirty,\r\n    /// Everything is Dirty, equivalent to SomeDirty(image_bounds)\r\n    AllDirty,\r\n    /// Some specific amount is dirty\r\n    SomeDirty(DeviceUintRect)\r\n}\r\n```\r\n\r\n\r\ncc @aosmond since it looks like they wrote a bunch of this code."},"comment":{"url":"https://api.github.com/repos/servo/webrender/issues/comments/426453438","html_url":"https://github.com/servo/webrender/issues/3155#issuecomment-426453438","issue_url":"https://api.github.com/repos/servo/webrender/issues/3155","id":426453438,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNjQ1MzQzOA==","user":{"login":"aosmond","id":1681373,"node_id":"MDQ6VXNlcjE2ODEzNzM=","avatar_url":"https://avatars3.githubusercontent.com/u/1681373?v=4","gravatar_id":"","url":"https://api.github.com/users/aosmond","html_url":"https://github.com/aosmond","followers_url":"https://api.github.com/users/aosmond/followers","following_url":"https://api.github.com/users/aosmond/following{/other_user}","gists_url":"https://api.github.com/users/aosmond/gists{/gist_id}","starred_url":"https://api.github.com/users/aosmond/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aosmond/subscriptions","organizations_url":"https://api.github.com/users/aosmond/orgs","repos_url":"https://api.github.com/users/aosmond/repos","events_url":"https://api.github.com/users/aosmond/events{/privacy}","received_events_url":"https://api.github.com/users/aosmond/received_events","type":"User","site_admin":false},"created_at":"2018-10-02T22:36:15Z","updated_at":"2018-10-02T22:36:15Z","author_association":"CONTRIBUTOR","body":"Another relevant piece perhaps is it makes the decision whether or not to perform the upload here:\r\n\r\nhttps://github.com/servo/webrender/blob/master/webrender/src/resource_cache.rs#L969\r\n\r\nIt can perform an upload if the dirty rect is Some *or* if the texture cache handle has been invalidated. In the latter case, we will get a dirty rect which is None in update_texture_cache, which means reupload the full thing."}}
{"actor":{"display_login":"aosmond"}}