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

Fix issues with secondary views and layer management. #180

Merged
merged 2 commits into from Jul 8, 2020

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented Jul 7, 2020

Fix issues with secondary views and layer management.

as i32,
height: (view_configuration.recommended_image_rect_height
/ SECONDARY_VIEW_DOWNSCALE) as i32,
width: view_configuration.recommended_image_rect_width as i32,

This comment has been minimized.

@Manishearth

Manishearth Jul 7, 2020

Member

why did we move the downscaling?

This comment has been minimized.

@asajeffrey

asajeffrey Jul 7, 2020

Author Member

Keeping the original value around seems cleaner, e.g. different layer types can use different scaling. But it's an inessential change, we can go back if we like.

This comment has been minimized.

@Manishearth

Manishearth Jul 8, 2020

Member

Ah, seems fine.

Err(e) => {
error!("Error waiting on frame: {:?}", e);
return None;
let (frame_state, secondary_state) = if self

This comment has been minimized.

@Manishearth

Manishearth Jul 7, 2020

Member

i wonder if a secondary_enabled can be stored on self instead: it's readonly

This comment has been minimized.

@asajeffrey

asajeffrey Jul 7, 2020

Author Member

Problem is it's needed by the views() function, so we'd end up duplicating it.

This comment has been minimized.

@Manishearth

Manishearth Jul 7, 2020

Member

Yes, but it's a boolean, so it's cheap and better than locking twice

This comment has been minimized.

@asajeffrey

asajeffrey Jul 7, 2020

Author Member

That's true, this double-locking is annoying.

This comment has been minimized.

@asajeffrey

asajeffrey Jul 7, 2020

Author Member

Fixed.

@Manishearth
Copy link
Member

Manishearth commented Jul 8, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jul 8, 2020

📌 Commit 1bad69b has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Jul 8, 2020

Testing commit 1bad69b with merge 88f287d...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 8, 2020

☀️ Test successful - checks-travis
Approved by: Manishearth
Pushing 88f287d to master...

@bors-servo bors-servo merged commit 88f287d into servo:master Jul 8, 2020
3 checks passed
3 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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.