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

acid2 has regressed #2061

Closed
metajack opened this issue Apr 7, 2014 · 6 comments
Closed

acid2 has regressed #2061

metajack opened this issue Apr 7, 2014 · 6 comments

Comments

@metajack
Copy link
Contributor

@metajack metajack commented Apr 7, 2014

Current master 7541b57 has a few different problems, around the top of the smiley.

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Apr 7, 2014

People are seeing problems with the eyes:
http://imgur.com/GMDntyj

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 7, 2014

I can reproduce this bug on 83aabe3 but not on 7ece5f9. In between these, there’s only the Rust upgrade #2041.

@june0cho
Copy link
Contributor

@june0cho june0cho commented May 24, 2014

Still reproduced.
If the first line of face(https://github.com/mozilla/servo/blob/master/src/test/html/acid2.html#L128) is commented out, then it looks good.
This line has the following style:

.picture p { position: fixed; margin: 0; padding: 0; border: 0; top: 9em; left: 11em; width: 140%; max-width: 4em; height: 8px; min-height: 1em; max-height: 2mm;
 background: black; border-bottom: 0.5em yellow solid; }
@mrobinson
Copy link
Member

@mrobinson mrobinson commented Oct 6, 2014

The issue here seems is that the missing bits (eyes and top of the forehead) are layerized, but those layers are not placed in the correct position. They seem to be layerized because they have siblings with position:fixed. The layer origins are 0, 36 and 36, 60. I've attached a screenshot of this with layer borders turned on, the two invisible layers are not position:fixed so are rendered nowhere near the face which is much further down the page. I think that they are invisible because there is no layer content at that position on the page.
shot

@mrobinson
Copy link
Member

@mrobinson mrobinson commented Oct 6, 2014

After applying the following patch applied in block.rs

         // If we got here, then we need a new layer.
         let layer_rect = self.base.position.union(&self.base.overflow);
         let size = Size2D(layer_rect.size.inline.to_nearest_px() as uint,
                           layer_rect.size.block.to_nearest_px() as uint);
-        let origin = Point2D(layer_rect.start.i.to_nearest_px() as uint,
-                             layer_rect.start.b.to_nearest_px() as uint);
+        //let origin = Point2D(layer_rect.start.i.to_nearest_px() as uint,
+        //                     layer_rect.start.b.to_nearest_px() as uint);
+        let origin = Point2D(self.base.abs_position.x.to_nearest_px() as uint,
+                             self.base.abs_position.y.to_nearest_px() as uint);

shot

The rendering still isn't correct, but I think that since layers are always added as children of the root layer, using the relative position is incorrect. I'm not sure exactly why the forehead is still incorrect, but I'll continue to investigate.

@mrobinson
Copy link
Member

@mrobinson mrobinson commented Oct 7, 2014

At first I was confused as to why the Acid2 reftest is passing. The Acid2 reftest is a special non-scrolling version of Acid2. The misplaced layers are still incorrectly placed, but because tiles are 512x512, the layer's top left tile covers the face completely. This means that the contents are rendered properly, just at an unexpected place in the tile. Failures appear when activating fitted tiles from rust-layers HEAD.

Here's a screenshot, where orange is layer boundaries and blue is tile boudaries:
shot

mrobinson added a commit to mrobinson/servo that referenced this issue Oct 8, 2014
Layers are currently all children of the root layer, so instead of
using coordinates relative to the parent flow we should use coordinates
relative to the page.

Fixes servo#2061.
bors-servo pushed a commit that referenced this issue Oct 8, 2014
Layers are currently all children of the root layer, so instead of
using coordinates relative to the parent flow we should use coordinates
relative to the page.

Fixes #2061.
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.

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