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

Servo will do reflow 3x at startup #5093

Closed
Adenilson opened this issue Feb 27, 2015 · 12 comments
Closed

Servo will do reflow 3x at startup #5093

Adenilson opened this issue Feb 27, 2015 · 12 comments

Comments

@Adenilson
Copy link
Contributor

@Adenilson Adenilson commented Feb 27, 2015

How to reproduce:

Start servo with ./servo --debug dump-display-list url

Expected result

It should dump the display list and print 'start printing display list' 3 times.

Further info

This points that reflow is being triggered at least 3 times. It is confirmed for both local and remote URLs, and happens in both OSX and Linux.

Maybe it has to do with window resize events forcing a new reflow?

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Feb 28, 2015

Yep, the patch on #5096 kind of confirms that the extra 2x reflows are thanks to window resize events.

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Mar 9, 2015

Loading CNN will have many reflow events. Working with the theory that they could be triggered by images, I started with a simple page that has several elements (script, style, p, div, link and an image) and confirmed that the forced reflows are bound to the presence of an image.

The test page: https://gist.github.com/Adenilson/15997c41a37a7ce29e85

What is a bit puzzling is that sometimes there will be a single reflowEvent and most frequently 2x reflows as seen in: https://gist.github.com/Adenilson/e068ab9a8a49ad345070

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Mar 9, 2015

Also see #5150 and #5079.

@metajack
Copy link
Contributor

@metajack metajack commented Mar 10, 2015

Based on @pcwalton's comments on IRC, it seems like the discrepancy between 1 and 2 reflows is when the image load completes.

This seems like two bugs in one. The first bug is that loading an image shouldn't necessarily reflow. It's quite possible the size of the image is already known and no layout computation needs to happen.

The second bug is that if the image isn't loaded, and it might reflow, we are doing extra reflow for no reason. In this case, I'm guessing it will use some default size since it doesn't know the real size, reflow, then reflow again when the size is known. This could explain the jiggling from #5083.

@metajack
Copy link
Contributor

@metajack metajack commented Mar 10, 2015

Or rather, it explains some of the jiggling. Because that title slide has no images and the other ones will all have display: none!

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Mar 12, 2015

Ok, doing some work in another issue, I just stumbled on this:
https://github.com/servo/servo/blob/master/components/script/dom/htmlimageelement.rs#L119

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Mar 17, 2015

I have a question… is the layout non deterministic? I’m seeing some different results while running servo (e.g. concerning the display lists).

For this page https://gist.github.com/Adenilson/850bc5260b2fcd218c9e

I faced this https://gist.github.com/Adenilson/d3e7b1556eb109db6121
and this https://gist.github.com/Adenilson/3b9a23348bdb59bcab2d
(the later one is less frequent).

Can the display list optimizer drop some items from time to time?

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Mar 17, 2015

Non-determinism in end results (as opposed to just scheduling) smells like bugs.

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Mar 17, 2015

That being said, what is consistent is 2 reflow events at startup (FirstLoad, WindowResize) e.g. running with -Z relayout-event.

And when there is a single image in the page, 2 other events (ReceivedReflowEvent + ReceivedReflowEvent).

@nox
Copy link
Member

@nox nox commented Sep 30, 2017

Is this still true? Can we close this?

@dralley
Copy link
Contributor

@dralley dralley commented Mar 24, 2020

Bump :)

@Adenilson
Copy link
Contributor Author

@Adenilson Adenilson commented Mar 24, 2020

I haven't worked on Servo for the last 5 years, my guess is that you can just close this issue.

@jdm jdm closed this Mar 24, 2020
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
6 participants
You can’t perform that action at this time.