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

Duplicate rendering when changing viewport coordinates #20847

Closed
paulrouget opened this issue May 22, 2018 · 9 comments
Closed

Duplicate rendering when changing viewport coordinates #20847

paulrouget opened this issue May 22, 2018 · 9 comments

Comments

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented May 22, 2018

STR:

HTML:

<style>
  body {
    background: red;
    height: 2000px;
  }
</style>

Change this in servo:

diff --git a/ports/servo/glutin_app/window.rs b/ports/servo/glutin_app/window.rs
index 436f7518ee..4c679c8095 100644
--- a/ports/servo/glutin_app/window.rs
+++ b/ports/servo/glutin_app/window.rs
@@ -700,7 +700,7 @@ impl WindowMethods for Window {
                 let (width, height) = window.get_inner_size().expect("Failed to get window inner size.");
                 let inner_size = (TypedSize2D::new(width as f32, height as f32) * dpr).to_u32();

-                let viewport = DeviceUintRect::new(TypedPoint2D::zero(), inner_size);
+                let viewport = DeviceUintRect::new(TypedPoint2D::new(50,50), TypedSize2D::new(300,300));

                 EmbedderCoordinates {
                     viewport: viewport,

Result:

screen shot 2018-05-22 at 15 16 41

@glennw any idea why the content doesn't stay in the top square? When we add a document (via add_document), are we supposed to use the size of the framebuffer of the size of the document?

@paulrouget paulrouget changed the title Duplicate rendering when change viewport coordinates Duplicate rendering when changing viewport coordinates May 22, 2018
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented May 22, 2018

/cc @gw3583

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented May 22, 2018

It only happen when the document overflows. I think the compositor code is fine. But layout must be missing something.

@gw3583 / @glennw are we supposed to also shift the layout coordinates?

@gw3583
Copy link
Contributor

@gw3583 gw3583 commented May 22, 2018

I don't think you should have to change any layout coordinates. Any ideas @kvark ?

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented May 23, 2018

I believe I need to set the document dimension in add_document. It appears to work better. I'm confused to why this is necessary as set_window_parameter is supposed to used to send the same information.

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented May 23, 2018

I believe I need to set the document dimension in add_document. It appears to work better.

Less content is rendered in the bottom part, but still, there's still a red square there.

@kvark
Copy link
Member

@kvark kvark commented May 23, 2018

Document scissoring appears to be broken right now (in our own "document" example). I'm looking into it.

@kvark kvark self-assigned this May 23, 2018
bors-servo added a commit to servo/webrender that referenced this issue May 28, 2018
Fix document placement and scissoring

This PR fixes our document example.
r? @gw3583

Edit: actually, still working on getting servo/servo#20847 fixed by this

<!-- 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/2770)
<!-- Reviewable:end -->
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented May 31, 2018

@kvark Updating WR doesn't look trivial (few changes in the API). Are you planning an update soon?

@kvark
Copy link
Member

@kvark kvark commented May 31, 2018

Tomorrow, according to @gw3583 👏

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Jul 5, 2018

This is fixed now. Thanks!

@paulrouget paulrouget closed this Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.