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

Move app_units::Au into api::units, re-export Au #3062

Closed
wants to merge 2 commits into from

Conversation

@fschutt
Copy link
Contributor

fschutt commented Sep 14, 2018

Fixes #3045

Now we can update app_units in one place (the webrender_api crate) instead of having to track the version of app_units in every tool. Also any dependencies that build upon webrender now don't have to worry about tracking what version of app_units webrender is using.

Also created a new create_app_units_from_px which does the required validation from pixel values to Au.


This change is Reviewable

@fschutt
Copy link
Contributor Author

fschutt commented Sep 14, 2018

It seems to fail because of a "license check issue" on Linux - I don't understand that, I didn't change anything about licenses. Maybe a style lint issue?

@gw3583
Copy link
Collaborator

gw3583 commented Sep 14, 2018

It's not clear to me what that means either - you might be able to get more info by running that step locally? (the taskcluster / appveyor config files should have the command info).

@bors-servo
Copy link
Contributor

bors-servo commented Sep 17, 2018

The latest upstream changes (presumably #3065) made this pull request unmergeable. Please resolve the merge conflicts.

@fschutt
Copy link
Contributor Author

fschutt commented Sep 17, 2018

The reason I couldn't run in offline was because it runs servo-tidy and I don't know where to find that servo-tidy script.

@kvark
Copy link
Member

kvark commented Sep 18, 2018

@fschutt maybe this is what it's really complaining about:

./webrender_api/src/units.rs:166: no newline at EOF

@fschutt fschutt force-pushed the fschutt:expose_app_units branch from 213934c to 02e41c3 Sep 19, 2018
@fschutt
Copy link
Contributor Author

fschutt commented Sep 19, 2018

Alright, seems to have fixed itself. Rebased on master to resolve the merge conflicts.

@kvark
Copy link
Member

kvark commented Sep 19, 2018

@fschutt could you squash the commits please? it is nice when each commit can be later investigated independently, which doesn't appear to be the case (e.g. "fixed Au imports").

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2018

The latest upstream changes (presumably #3075) made this pull request unmergeable. Please resolve the merge conflicts.

pcwalton and others added 2 commits Sep 18, 2018
`ClipItem::new_box_shadow()`.

If no blur is present, `DisplayListFlattener::add_box_shadow()` calls
`ClipItem::new_rounded_rect()` in its fast path, which calls
`ensure_no_corner_overlap()`. However, in the slow path, which is used when
blur is present, `ClipItem::new_box_shadow()` is called instead. Prior to this
patch, `ClipItem::new_box_shadow()` neglected to call
`ensure_no_corner_overlap()`. This is why the bug manifested itself only when
blur was present.

Closes #2581.
Fixes #3045

Now we can update app_units in one place (the webrender_api
crate) instead of having to track the version of app_units
in every tool. Also any dependencies that build upon webrender
now don't have to worry about tracking what version of app_units
webrender is using.

Also created a new `create_app_units_from_px` which does the
required validation from pixel values to Au.

Adressed style lint issues

Rebased changes from servo:master + fixed Au imports
@fschutt fschutt force-pushed the fschutt:expose_app_units branch from 02e41c3 to c8f8420 Sep 23, 2018
@fschutt
Copy link
Contributor Author

fschutt commented Mar 24, 2019

I think this can be closed, since Au is now exported under units::Au.

@fschutt fschutt closed this Mar 24, 2019
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

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