-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add LBSE blog entry (WIP) #226
Conversation
bb7b884
to
f195d61
Compare
|
||
In **October 2023** we started the project, mostly by thinking of the design and roadmap. Also we did some general SVG bug fixing. First, visual overflow computation for SVG renderers was <a href="https://commits.webkit.org/268981@main">corrected</a>, which fixed quite some SVG pixel tests. Various visual bugs were also fixed like <a href="https://commits.webkit.org/269034@main">unnecessary repainting when viewBox is used on <svg> elements</a> and <a href="https://commits.webkit.org/269360@main">incorrect clipping for outermost <svg> elements</a> | ||
|
||
Also in the same month, we attended the <a href="https://docs.webkit.org/Other/Contributor%20Meetings/ContributorMeeting2023.html">WebKit Contributors</a> meeting, where we presented a talk on the LBSE (slides are <a href="https://teams.pages.igalia.com/webkit/contributors-meeting-presentations/2023/igalia-slides/integrating-the-new-LBSE.html">here</a>) and the feedback on LBSE at the meeting was very positive. Giving the talk early on actually helped us since we needed to have a good design in place. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally we link to teams.pages.igalia do we? Normally for talks we post and link them on slideshare I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean we generally don’t link to teams.pages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know. The reference itself is very useful I think to give the blog some more context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sorry typo - I indeed meant we don't generally hotlink to those. We should probably upload to slideshare (I would be surprised if it isn't already there?) and link to that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikolaszimmermann we still need a slideshare :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the URL from https://www.slideshare.net/igalia/integrating-the-new-layerbased-svg-engine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks. D'oh, I think I saw that page before but forgot.
There are some grammatical edits I’d like to make, but it looks like there are still some bits to be written (Wix, conclusion). Please ping me when the text is complete and I’ll be happy to do an edit pass! |
Yeah, Niko will have a look and likely can add some words about Wix, we'll ping you after, thanks for taking a look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, I will try to expand on the Wix section.
|
||
## Support for gradients | ||
|
||
In early **January 2024** support for SVG gradients was <a href="https://commits.webkit.org/272653@main">upstreamed</a>. This is a kind of SVG resource that is a bit different to the previously implemented clipping paths/masks because it is a <a href="">paint server</a>, so a helper class for that called `SVGPaintServerHandling` and a base class `RenderSVGResourcePaintServer` were introduced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"that is a bit different" -> Another sentence to explain paint servers and why they are different to masks/clippers wrt. to invalidation etc. would be helpful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a sentence, I am not sure if the difference is really that big, or I misremember the invalidation logic (was a few months ago now).
9557f61
to
018f42e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there IMHO.
on performance, however we know there are likely optimizations possible in <code>RenderLayer</code> usage, both in reducing the number of <code>RenderLayer</code> objects we create in certain situations as well as a possible reduction | ||
in complexity of <code>RenderLayer</code> for LBSE usage. | ||
|
||
## Conclusion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove that section and merge the sentence into Next steps and eventually rename it to Outlook?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, personally I am not such a fan of "Outlook", if others want it I can change it to that of course.
For the short and mid-term, the plan is to make LBSE at least as good as legacy in regards to test coverage, i.e. no test should pass in legacy but not in LBSE. We have made a lot of progress over the | ||
last 7 months just because of the amount of SVG resources that were implemented, but for example we will need to have SVG filters in place to pass this goal. | ||
|
||
Another goal is to make sure LBSE qualifies for all security requirements, we are already adopting a lot of good <a href="https://github.com/WebKit/WebKit/wiki/Smart-Pointer-Usage-Guidelines">smart pointer practices</a> to support this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads a bit strange to me too... sounds like there's something missing between the comma and the "we are".
How about this:
Another goal is to make sure LBSE qualifies for all security requirements, as that would be a blocker otherwise before the current engine can be replaced. Fortunataly, we are already taking this into account in several ways to support this, such as adopting a lot of good smart pointer practices, for instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to mention smart pointers, fixed.
25274f8
to
a1bd174
Compare
Igalia logo added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
null
Site preview: https://igalia.github.io/wpewebkit.org/lbseblog/