From b36ef2cf858edd9603c05a99d68e9b5b2a88acd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Mon, 25 May 2026 01:49:04 -0300 Subject: [PATCH] Fix background draw in atomic boxes. Fix in selector parser. --- .agent/rules/html-layout-architecture.md | 10 +++ .../assets/html/ensoft/background.css | 16 +++++ .../assets/html/ensoft/background.html | 13 ++++ bin/unit_tests/assets/html/ensoft/ensoft.css | 4 -- include/eepp/graphics/richtext.hpp | 6 +- src/eepp/graphics/richtext.cpp | 31 ++++++-- src/eepp/ui/css/stylesheetselector.cpp | 4 ++ src/eepp/ui/uirichtext.cpp | 28 +++++--- src/tests/unit_tests/uihtml_tests.cpp | 70 +++++++++++++++++++ 9 files changed, 159 insertions(+), 23 deletions(-) create mode 100644 bin/unit_tests/assets/html/ensoft/background.css create mode 100644 bin/unit_tests/assets/html/ensoft/background.html diff --git a/.agent/rules/html-layout-architecture.md b/.agent/rules/html-layout-architecture.md index 8c1288448..a49a6385c 100644 --- a/.agent/rules/html-layout-architecture.md +++ b/.agent/rules/html-layout-architecture.md @@ -61,6 +61,16 @@ Layout math is separated from widgets into `UILayouter` implementations: `RichText::updateLayout()` performs line wrapping and inline formatting. `BlockLayouter::positionRichTextChildren()` then consumes `RichText::RenderSpan`s and assigns pixel positions/sizes back to DOM widgets. +### Inline Paint Ownership + +Inline text-span CSS is split across three layers: + +- `UITextSpan::applyProperty()` owns CSS-to-style mapping for inline text properties. For `background-color` it sets the span font background color because inline spans are normally painted by the parent rich-text stream, not by `UITextSpan::draw()`. +- `UIRichText::rebuildRichText()` is the DOM-to-rich-text bridge. It converts inline `UITextSpan` widgets into `RichText::InlineItem::Box` entries, carrying margin, padding, border, text decoration, background color, optional background image/layers, and optional rounded `UIBackgroundDrawable`. +- `Graphics::RichText::draw()` owns the actual inline fragment painting. It paints box background color first, then background image/layers, then borders, and finally text/atomic content. When a span has both `background-color` and `background-image`, the color fill must still render behind the image layer. When the span has `border-radius`, use the span `UIBackgroundDrawable` for the color fill so rounded corners are preserved. + +Atomic inline widgets (`RichText::RenderSpan::Type::AtomicBox`) do not call `UITextSpan::draw()`. Any visual style that belongs to an atomic inline-level box must therefore be carried through the `UIRichText::rebuildRichText()` metadata and painted by `RichText`, or it will be lost. + ### UITextNode `UITextNode` is a lightweight non-rendering node for raw parsed text (`node_pcdata`). Its text is extracted during `rebuildRichText()` and rendered by the parent `UIRichText`. After wrapping, `BlockLayouter` assigns it position and size for debugging and hit-box accounting, but `UITextNode::draw()` remains a no-op. diff --git a/bin/unit_tests/assets/html/ensoft/background.css b/bin/unit_tests/assets/html/ensoft/background.css new file mode 100644 index 000000000..304891d50 --- /dev/null +++ b/bin/unit_tests/assets/html/ensoft/background.css @@ -0,0 +1,16 @@ +#bar #rss { + margin-top: 20px; + margin-left: 20px; +} +#bar #rss a { + background: url("rss.png") no-repeat 8px center; + padding: 5px 10px 5px 24px; + text-decoration: none; + font-size: 12px; + border-radius: 4px; + background-color: #d0d0d0; + color: white; +} +#bar #rss a:hover { + background-color: red; +} diff --git a/bin/unit_tests/assets/html/ensoft/background.html b/bin/unit_tests/assets/html/ensoft/background.html new file mode 100644 index 000000000..10e1b3178 --- /dev/null +++ b/bin/unit_tests/assets/html/ensoft/background.html @@ -0,0 +1,13 @@ + + + + + + +
+
+ +
+
+ + diff --git a/bin/unit_tests/assets/html/ensoft/ensoft.css b/bin/unit_tests/assets/html/ensoft/ensoft.css index 45d9cb9bb..db12e572d 100644 --- a/bin/unit_tests/assets/html/ensoft/ensoft.css +++ b/bin/unit_tests/assets/html/ensoft/ensoft.css @@ -256,7 +256,6 @@ a img { cursor: pointer; background-color: black; border-radius: 5px; - behavior: url('https://ensoft.dev/assets/js/PIE.php'); font-size: 16px; color: white !important; } @@ -403,7 +402,6 @@ a img { border: 5px solid #f0f0f0; padding: 10px; border-radius: 10px; - behavior: url('https://ensoft.dev/assets/js/PIE.php'); } .blog_post .markdown blockquote { border-left: 5px solid #505050; @@ -482,7 +480,6 @@ a img { cursor: pointer; background-color: black; border-radius: 5px; - behavior: url('https://ensoft.dev/assets/js/PIE.php'); font-size: 16px; color: white !important; float: none; @@ -531,7 +528,6 @@ a img { font-weight: bold; background-color: red; border-radius: 5px; - behavior: url('https://ensoft.dev/assets/js/PIE.php'); } #minimap { display: none; diff --git a/include/eepp/graphics/richtext.hpp b/include/eepp/graphics/richtext.hpp index 63c590798..57d63cd31 100644 --- a/include/eepp/graphics/richtext.hpp +++ b/include/eepp/graphics/richtext.hpp @@ -307,6 +307,7 @@ class EE_API RichText : public Drawable { Color backgroundColor{ Color::Transparent }; Float borderWidth{ 0 }; Color borderColor{ Color::Transparent }; + Drawable* backgroundColorDrawable{ nullptr }; Drawable* backgroundDrawable{ nullptr }; Drawable* borderDrawable{ nullptr }; bool backgroundDrawableUsesFragmentColor{ false }; @@ -370,6 +371,7 @@ class EE_API RichText : public Drawable { Color backgroundColor{ Color::Transparent }; Float borderWidth{ 0 }; Color borderColor{ Color::Transparent }; + Drawable* backgroundColorDrawable{ nullptr }; Drawable* backgroundDrawable{ nullptr }; Drawable* borderDrawable{ nullptr }; bool backgroundDrawableUsesFragmentColor{ false }; @@ -388,8 +390,8 @@ class EE_API RichText : public Drawable { const BaselineAlignValue& baselineAlign, const Color& backgroundColor = Color::Transparent, Float borderWidth = 0, const Color& borderColor = Color::Transparent, Uint32 textDecoration = 0, - InlineSource source = {}, Drawable* backgroundDrawable = nullptr, - Drawable* borderDrawable = nullptr, + InlineSource source = {}, Drawable* backgroundColorDrawable = nullptr, + Drawable* backgroundDrawable = nullptr, Drawable* borderDrawable = nullptr, bool backgroundDrawableUsesFragmentColor = false ); /** Close the current inline box scope. */ diff --git a/src/eepp/graphics/richtext.cpp b/src/eepp/graphics/richtext.cpp index 09b8b9b14..28e4e0af2 100644 --- a/src/eepp/graphics/richtext.cpp +++ b/src/eepp/graphics/richtext.cpp @@ -86,6 +86,25 @@ RichText::RichText() : Drawable( Drawable::RICHTEXT ) {} RichText::~RichText() {} +static void drawInlineFragmentBackgroundColor( const RichText::InlineFragment& fragment, + const Rectf& rect, Float rotation ) { + if ( fragment.backgroundColor == Color::Transparent || + fragment.backgroundDrawableUsesFragmentColor ) + return; + + if ( fragment.backgroundColorDrawable != nullptr ) { + const Color oldColor = fragment.backgroundColorDrawable->getColor(); + fragment.backgroundColorDrawable->setColor( fragment.backgroundColor ); + fragment.backgroundColorDrawable->draw( rect.getPosition(), rect.getSize() ); + fragment.backgroundColorDrawable->setColor( oldColor ); + return; + } + + Primitives p; + p.setColor( fragment.backgroundColor ); + p.drawRectangle( rect, rotation, Vector2f::One ); +} + void RichText::draw() { draw( mPosition.x, mPosition.y ); } @@ -125,6 +144,7 @@ void RichText::draw( const Float& X, const Float& Y, const Vector2f& scale, cons fragment.paintBounds != Rectf::Zero ? fragment.paintBounds : fragment.bounds; Rectf rect( localPaintBounds.getPosition() * scale + Vector2f( X, Y ), localPaintBounds.getSize() * scale ); + drawInlineFragmentBackgroundColor( fragment, rect, rotation ); if ( fragment.backgroundDrawable != nullptr ) { if ( fragment.backgroundDrawableUsesFragmentColor && fragment.backgroundColor != Color::Transparent ) { @@ -135,10 +155,6 @@ void RichText::draw( const Float& X, const Float& Y, const Vector2f& scale, cons } else { fragment.backgroundDrawable->draw( rect.getPosition(), rect.getSize() ); } - } else if ( fragment.backgroundColor != Color::Transparent ) { - Primitives p; - p.setColor( fragment.backgroundColor ); - p.drawRectangle( rect, rotation, Vector2f::One ); } if ( fragment.borderDrawable != nullptr ) { fragment.borderDrawable->draw( rect.getPosition(), rect.getSize() ); @@ -1828,6 +1844,7 @@ class RichTextInlineLayouter { fragment.backgroundColor = acc.box->backgroundColor; fragment.borderWidth = acc.box->borderWidth; fragment.borderColor = acc.box->borderColor; + fragment.backgroundColorDrawable = acc.box->backgroundColorDrawable; fragment.backgroundDrawable = acc.box->backgroundDrawable; fragment.borderDrawable = acc.box->borderDrawable; fragment.backgroundDrawableUsesFragmentColor = @@ -1924,8 +1941,9 @@ void RichText::addLineBreak() { void RichText::pushInlineBox( const Rectf& margin, const Rectf& padding, Float lineHeight, const BaselineAlignValue& baselineAlign, const Color& backgroundColor, Float borderWidth, const Color& borderColor, Uint32 textDecoration, - InlineSource source, Drawable* backgroundDrawable, - Drawable* borderDrawable, bool backgroundDrawableUsesFragmentColor ) { + InlineSource source, Drawable* backgroundColorDrawable, + Drawable* backgroundDrawable, Drawable* borderDrawable, + bool backgroundDrawableUsesFragmentColor ) { InlineItem item; InlineItem::Box box; box.source = source; @@ -1936,6 +1954,7 @@ void RichText::pushInlineBox( const Rectf& margin, const Rectf& padding, Float l box.backgroundColor = backgroundColor; box.borderWidth = borderWidth; box.borderColor = borderColor; + box.backgroundColorDrawable = backgroundColorDrawable; box.backgroundDrawable = backgroundDrawable; box.borderDrawable = borderDrawable; box.backgroundDrawableUsesFragmentColor = backgroundDrawableUsesFragmentColor; diff --git a/src/eepp/ui/css/stylesheetselector.cpp b/src/eepp/ui/css/stylesheetselector.cpp index 8f489b90c..4dc391ca7 100644 --- a/src/eepp/ui/css/stylesheetselector.cpp +++ b/src/eepp/ui/css/stylesheetselector.cpp @@ -33,6 +33,10 @@ void removeExtraSpaces( std::string& string ) { String::replaceAll( string, " | ", "|" ); String::replaceAll( string, " + ", "+" ); String::replaceAll( string, " ~ ", "~" ); + String::replaceAll( string, " >", ">" ); + String::replaceAll( string, " |", "|" ); + String::replaceAll( string, " +", "+" ); + String::replaceAll( string, " ~", "~" ); } void StyleSheetSelector::addSelectorRule( diff --git a/src/eepp/ui/uirichtext.cpp b/src/eepp/ui/uirichtext.cpp index 23afaa3ec..7df4da6ff 100644 --- a/src/eepp/ui/uirichtext.cpp +++ b/src/eepp/ui/uirichtext.cpp @@ -1033,6 +1033,7 @@ static Color getInlineBorderColor( UIWidget* widget ) { } struct InlineBackgroundDrawable { + Drawable* colorDrawable{ nullptr }; Drawable* drawable{ nullptr }; bool usesFragmentColor{ false }; }; @@ -1044,11 +1045,16 @@ static InlineBackgroundDrawable getInlineBackgroundDrawable( UIWidget* widget, return {}; UINodeDrawable* background = widget->getBackground(); - if ( background->getBackgroundColor() != Color::Transparent || background->hasDrawableLayers() ) - return { background, false }; + const bool hasRoundedColorBackground = + backgroundColor != Color::Transparent && background->getBackgroundDrawable().hasRadius(); + if ( background->hasDrawableLayers() ) + return { hasRoundedColorBackground ? &background->getBackgroundDrawable() : nullptr, + background, false }; + if ( background->getBackgroundColor() != Color::Transparent ) + return { nullptr, background, false }; - if ( backgroundColor != Color::Transparent && background->getBackgroundDrawable().hasRadius() ) - return { &background->getBackgroundDrawable(), true }; + if ( hasRoundedColorBackground ) + return { nullptr, &background->getBackgroundDrawable(), true }; return {}; } @@ -1297,13 +1303,13 @@ void UIRichText::rebuildRichText( UILayout* container, RichText& richText, Intri auto backgroundDrawable = getInlineBackgroundDrawable( span, span->getFontStyleConfig().BackgroundColor ); - richText.pushInlineBox( margin, padding, spanLineHeight, - toRichTextBaselineAlign( span->getBaselineAlign() ), - span->getFontStyleConfig().BackgroundColor, borderWidth, - borderColor, span->getTextDecoration(), - toRichTextWidgetSource( span ), backgroundDrawable.drawable, - getInlineBorderDrawable( span ), - backgroundDrawable.usesFragmentColor ); + richText.pushInlineBox( + margin, padding, spanLineHeight, + toRichTextBaselineAlign( span->getBaselineAlign() ), + span->getFontStyleConfig().BackgroundColor, borderWidth, borderColor, + span->getTextDecoration(), toRichTextWidgetSource( span ), + backgroundDrawable.colorDrawable, backgroundDrawable.drawable, + getInlineBorderDrawable( span ), backgroundDrawable.usesFragmentColor ); inlineBoxDepth++; if ( hasOwnText ) { diff --git a/src/tests/unit_tests/uihtml_tests.cpp b/src/tests/unit_tests/uihtml_tests.cpp index aa91fdab9..eb836ad7e 100644 --- a/src/tests/unit_tests/uihtml_tests.cpp +++ b/src/tests/unit_tests/uihtml_tests.cpp @@ -2390,6 +2390,76 @@ UTEST( UIBackground, imageAtlasPositioning ) { Engine::destroySingleton(); } +UTEST( UIBackground, inlineSpanColorRendersBehindBackgroundImage ) { + auto win = Engine::instance()->createWindow( + WindowSettings( 320, 160, "Inline Background Color Test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + FileSystem::changeWorkingDirectory( Sys::getProcessPath() ); + + UI::UISceneNode* sceneNode = init_test_inline_block(); + sceneNode->setURI( "file://" + Sys::getProcessPath() + "assets/html/ensoft/" ); + + std::string html; + FileSystem::fileGet( "assets/html/ensoft/background.html", html ); + sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( html ) ); + win->setClearColor( Color::White ); + + win->getInput()->update(); + SceneManager::instance()->update(); + + win->clear(); + SceneManager::instance()->draw(); + win->display(); + + auto anchors = sceneNode->getRoot()->querySelectorAll( "#rss a" ); + ASSERT_EQ( anchors.size(), (size_t)1 ); + auto* anchor = anchors.front()->asType(); + ASSERT_TRUE( anchor != nullptr ); + EXPECT_EQ( anchor->getFontBackgroundColor().getValue(), + Color::fromString( "#d0d0d0" ).getValue() ); + ASSERT_TRUE( anchor->getBackground() != nullptr ); + ASSERT_TRUE( anchor->getBackground()->hasDrawableLayers() ); + ASSERT_TRUE( anchor->getBackground()->getBackgroundDrawable().hasRadius() ); + + bool foundRoundedLayeredFragment = false; + sceneNode->getRoot()->forEachNode( [&]( Node* node ) { + if ( foundRoundedLayeredFragment || !node->isType( UI_TYPE_RICHTEXT ) ) + return; + for ( const auto& fragment : + node->asType()->getRichText().getInlineFragments() ) { + if ( fragment.type == RichText::InlineFragment::Type::Box && + fragment.source.type == RichText::InlineSourceType::Widget && + fragment.source.ptr == anchor ) { + foundRoundedLayeredFragment = true; + EXPECT_TRUE( fragment.backgroundColorDrawable != nullptr ); + EXPECT_TRUE( fragment.backgroundDrawable != nullptr ); + EXPECT_EQ( fragment.backgroundColor.getValue(), + Color::fromString( "#d0d0d0" ).getValue() ); + break; + } + } + } ); + EXPECT_TRUE( foundRoundedLayeredFragment ); + + const Rectf rect = anchor->getScreenRect(); + ASSERT_GT( rect.getWidth(), 12.f ); + ASSERT_GT( rect.getHeight(), 4.f ); + + Image image = win->getFrontBufferImage(); + const Uint32 sampleX = static_cast( eefloor( rect.Right - 5.f ) ); + const Uint32 sampleY = static_cast( eefloor( rect.Top + rect.getHeight() * 0.5f ) ); + ASSERT_LT( sampleX, image.getWidth() ); + ASSERT_LT( sampleY, image.getHeight() ); + + const Color pixel = image.getPixel( sampleX, sampleY ); + EXPECT_NEAR( pixel.r, 0xd0, 4 ); + EXPECT_NEAR( pixel.g, 0xd0, 4 ); + EXPECT_NEAR( pixel.b, 0xd0, 4 ); + + Engine::destroySingleton(); +} + UTEST( UIBackground, imageAtlasPositioningPixelDensity2 ) { auto win = Engine::instance()->createWindow( WindowSettings( 1024, 653, "Background Atlas Test PD2", WindowStyle::Default,