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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb+LibGfx: A first pass at supporting (linear) gradients! #14564

Merged
merged 9 commits into from Jul 17, 2022

Conversation

MacDue
Copy link
Member

@MacDue MacDue commented Jul 12, 2022

Was bored, needed a new yak... Ran out of border-radius stuff 馃槩

image

This PR implements parsing for linear-gradient(), along with the the necessary plumbing, and some very basic painting.

The parsing is should work for everything defined in the W3 spec https://drafts.csswg.org/css-images/#linear-gradients, though there seems to be some issues there as multi-position stop points are not defined (though are demonstrated on MDN).
Though we can't paint all but the simplest of gradients yet, so this is not a huge issue yet.

P.S. The CSS parsing and style value side of LibWeb is slightly unfamiliar territory for me, so might not be optimal :^).

Copy link
Member

@AtkinsSJ AtkinsSJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! 馃憖 Had a little time to look at the first commit, will look at the rest later.

Userland/Libraries/LibWeb/CSS/StyleValue.cpp Outdated Show resolved Hide resolved
Userland/Libraries/LibWeb/CSS/StyleValue.h Outdated Show resolved Hide resolved
Userland/Libraries/LibWeb/CSS/StyleValue.h Outdated Show resolved Hide resolved
Userland/Libraries/LibWeb/CSS/StyleValue.h Outdated Show resolved Hide resolved
Userland/Libraries/LibWeb/Forward.h Outdated Show resolved Hide resolved
@AtkinsSJ AtkinsSJ self-requested a review July 12, 2022 12:01
@MacDue MacDue force-pushed the linear_gradients_book_1 branch 3 times, most recently from 8e29939 to e228be5 Compare July 12, 2022 15:34
This moves the logic from ColorStyleValue::to_string() to a standalone
function.
@MacDue
Copy link
Member Author

MacDue commented Jul 12, 2022

Rebased for the StringView changes from #14555

Copy link
Member

@AtkinsSJ AtkinsSJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few changes, but this is good stuff. :^)

One issue I'm not sure about, which doesn't have to be fixed here, is how to deal with gradients being allowed anywhere an image is. As written, you'd have to manually enable gradients for each property with specific painting code for them, which feels awkward. Maybe ImageStyleValue and LinearGradientStyleValue can both implement some interface for getting their inherent dimensions and painting them into a rectangle.

Userland/Libraries/LibWeb/CSS/StyleValue.cpp Outdated Show resolved Hide resolved
Userland/Libraries/LibWeb/CSS/StyleValue.h Outdated Show resolved Hide resolved
Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp Outdated Show resolved Hide resolved
Base/res/html/misc/welcome.html Show resolved Hide resolved
Userland/Libraries/LibWeb/Painting/BackgroundPainting.cpp Outdated Show resolved Hide resolved
@MacDue
Copy link
Member Author

MacDue commented Jul 15, 2022

Done the easy changes and added some FIXMEs for the rest. I think trying to make gradients and images share a common API for painting would be good, but I think that should be tackled separately since I think it'll be a bit tricky :^)

@Lubrsi
Copy link
Member

Lubrsi commented Jul 16, 2022

It seems we must also support the -webkit prefix for linear-gradient. For example, the GitHub landing page only uses the -webkit prefixed version with no fallback:
Screenshot from 2022-07-16 14-51-56

This should parse linear-gradient()s as defined in the W3 spec
https://drafts.csswg.org/css-images/#linear-gradients.

Note: This currently cannot parse multi-position color stops,
these are shown on MDN and work in Firefox and Chrome, though do
not seem to be defined in the spec.

See: https://developer.mozilla.org/en-US/docs/Web/CSS/gradient/linear-gradient#gradient_with_multi-position_color_stops

P.s. This also allows -webkit-linear-gradient for compatibility.
This is just a quick test that everything is working. Currently
it paints the gradients with the existing
painter.fill_rect_with_gradient(). This can only handle two-color
orthogonal gradients.
Previously the color blending and alpha blending were working from
opposite sides.
@MacDue
Copy link
Member Author

MacDue commented Jul 16, 2022

The parser should recognise the -webkit variant now too :^)

@AtkinsSJ
Copy link
Member

LGTM! (Look, Gradients; Their Majesty!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants