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

Separate interning borders #3429

Merged
merged 2 commits into from Dec 18, 2018
Merged

Conversation

@djg
Copy link
Contributor

djg commented Dec 18, 2018

In support of #3385, extract normal and image borders. This patch is based up PR #3393 and requires that to land first.


This change is Reviewable

@gw3583
Copy link
Collaborator

gw3583 commented Dec 18, 2018

#3393 has landed now, so it should be possible to rebase this and remove those commits.

@djg djg force-pushed the djg:separate_interning_borders branch from b61f847 to 81bbbce Dec 18, 2018
Copy link
Collaborator

gw3583 left a comment

Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @djg)


webrender/src/border.rs, line 16 at r1 (raw file):

use prim_store::{BorderSegmentInfo, BrushSegment, NinePatchDescriptor};
use prim_store::{EdgeAaSegmentMask, ScrollNodeAndClipChain};
use prim_store::borders::NormalBorder;

I wonder if we should call these something like NormalBorderDescriptor or something like that to avoid naming conflicts with the API structs? Not something we need to do now, but perhaps consider as a follow up?

@gw3583
Copy link
Collaborator

gw3583 commented Dec 18, 2018

One question - can either ignore or do as a follow up, if that's easier.

Apart from that, looks good to me - do we want a try / talos run before merging? (I suspect this might have a decent perf win on dl_mutate, given the reduction in primitive key size).

@bors-servo delegate+

@bors-servo
Copy link
Contributor

bors-servo commented Dec 18, 2018

✌️ @djg can now approve this pull request

@djg
Copy link
Contributor Author

djg commented Dec 18, 2018

Copy link
Contributor Author

djg left a comment

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gw3583)


webrender/src/border.rs, line 16 at r1 (raw file):

Previously, gw3583 (Glenn Watson) wrote…

I wonder if we should call these something like NormalBorderDescriptor or something like that to avoid naming conflicts with the API structs? Not something we need to do now, but perhaps consider as a follow up?

NormalBorderPrim?

@djg djg force-pushed the djg:separate_interning_borders branch 3 times, most recently from dbef778 to cb4c247 Dec 18, 2018
@gw3583
gw3583 approved these changes Dec 18, 2018
@djg
Copy link
Contributor Author

djg commented Dec 18, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Dec 18, 2018

📌 Commit cb4c247 has been approved by gw

@bors-servo
Copy link
Contributor

bors-servo commented Dec 18, 2018

Testing commit cb4c247 with merge d554558...

bors-servo added a commit that referenced this pull request Dec 18, 2018
Separate interning borders

In support of #3385, extract normal and image borders. This patch is based up PR #3393 and requires that to land first.

<!-- 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/3429)
<!-- Reviewable:end -->
@djg djg force-pushed the djg:separate_interning_borders branch from cb4c247 to ce020d4 Dec 18, 2018
@djg
Copy link
Contributor Author

djg commented Dec 18, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Dec 18, 2018

📌 Commit ce020d4 has been approved by gw

bors-servo added a commit that referenced this pull request Dec 18, 2018
Separate interning borders

In support of #3385, extract normal and image borders. This patch is based up PR #3393 and requires that to land first.

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

bors-servo commented Dec 18, 2018

Testing commit ce020d4 with merge 14df2e4...

@bors-servo
Copy link
Contributor

bors-servo commented Dec 18, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw
Pushing 14df2e4 to master...

@bors-servo bors-servo merged commit ce020d4 into servo:master Dec 18, 2018
3 of 4 checks passed
3 of 4 checks passed
code-review/reviewable 9 files, 1 discussion left (gw3583)
Details
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@djg djg deleted the djg:separate_interning_borders branch Dec 18, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 19, 2018
…69d26e35b161 (WR PR #3429). r=kats

servo/webrender#3429

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

--HG--
extra : moz-landing-system : lando
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Dec 19, 2018
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…69d26e35b161 (WR PR #3429). r=kats

servo/webrender#3429

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

UltraBlame original commit: 070bb58d4d49c800a3221255eeaf202eaccb9a86
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…69d26e35b161 (WR PR #3429). r=kats

servo/webrender#3429

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

UltraBlame original commit: 070bb58d4d49c800a3221255eeaf202eaccb9a86
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
…69d26e35b161 (WR PR #3429). r=kats

servo/webrender#3429

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

UltraBlame original commit: 070bb58d4d49c800a3221255eeaf202eaccb9a86
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

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