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

Fix some picture caching edge cases exposed by gecko reftests. #3415

Merged
merged 1 commit into from Dec 17, 2018

Conversation

Projects
None yet
3 participants
@gw3583
Copy link
Collaborator

gw3583 commented Dec 14, 2018

This change is Reviewable

@gw3583

This comment has been minimized.

Copy link
Collaborator

gw3583 commented Dec 14, 2018

r? @kvark or @nical

@kvark

kvark approved these changes Dec 14, 2018

Copy link
Member

kvark left a comment

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gw3583)


webrender/src/picture.rs, line 413 at r1 (raw file):

        // picture rect, and then we re-enable it, force an update
        // of all primitive dependencies.
        if self.tile_rect.size == TileSize::zero() {

nit: there is also is_empty_or_negative()


webrender/src/picture.rs, line 706 at r1 (raw file):

        //           we should tidy this up to take advantage of this!
        let mut in_visible_cluster = false;
        for ci in prim_instance.cluster_range.start .. prim_instance.cluster_range.end {

this could be rewritten simpler as

let in_visible_cluster = prim_instance.cluster_range
  .clone()
  .any(|ci| prim_list.clusters[prim_list.prim_cluster_map[ci as usize].0 as usize].is_visible);

webrender/src/picture.rs, line 2142 at r1 (raw file):

            // but it's simple and works around this edge case for now.
            if let Some(ref mut tile_cache) = self.tile_cache {
                if surface_rect.size.width > 65536.0 ||

nit: make it a constant?

@gw3583 gw3583 force-pushed the gw3583:pc-fixes branch from 63f2b8c to fcc15de Dec 16, 2018

@gw3583
Copy link
Collaborator

gw3583 left a comment

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @kvark)


webrender/src/picture.rs, line 413 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

nit: there is also is_empty_or_negative()

Fixed.


webrender/src/picture.rs, line 706 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

this could be rewritten simpler as

let in_visible_cluster = prim_instance.cluster_range
  .clone()
  .any(|ci| prim_list.clusters[prim_list.prim_cluster_map[ci as usize].0 as usize].is_visible);

I've left as is, for now - I intend to remove the multiple cluster support (it's unused) this week, which will simplify how this code works.


webrender/src/picture.rs, line 2142 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

nit: make it a constant?

Done

@gw3583

This comment has been minimized.

Copy link
Collaborator

gw3583 commented Dec 16, 2018

@bors-servo r=kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 16, 2018

📌 Commit fcc15de has been approved by kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 16, 2018

⌛️ Testing commit fcc15de with merge edc5440...

bors-servo added a commit that referenced this pull request Dec 16, 2018

Auto merge of #3415 - gw3583:pc-fixes, r=kvark
Fix some picture caching edge cases exposed by gecko reftests.

<!-- 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/3415)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 17, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing edc5440 to master...

@bors-servo bors-servo merged commit fcc15de into servo:master Dec 17, 2018

3 of 4 checks passed

code-review/reviewable 1 file, 2 discussions left (kvark)
Details
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 17, 2018

Bug 1514739 - Update webrender to commit edc54403140bc178c86f81be318b…
…9d97087c8fd1 (WR PR #3415). r=kats

servo/webrender#3415

Differential Revision: https://phabricator.services.mozilla.com/D14738

--HG--
extra : moz-landing-system : lando

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Dec 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment