Optimize CSS selector traversal and fix sibling combinators:

- inline hot-path node and stylesheet traversal accessors
  - correct adjacent and general sibling matching direction
  - preserve inverse sibling operator behavior
  - add selector traversal and sibling regression tests
This commit is contained in:
Martín Lucas Golini
2026-07-10 01:26:06 -03:00
parent 2bc18f0ded
commit 6b4179b896
7 changed files with 154 additions and 97 deletions

View File

@@ -60,6 +60,8 @@ Do not start with a large rewrite. Measure after each phase.
# Phase 1: Inline Tiny Hot-Path Accessors
**Status: Implemented**
## Motivation
CSS selector matching repeatedly calls small accessors and type checks while walking parents/siblings and matching selector rules. These functions are ideal candidates for header inlining.
@@ -189,6 +191,8 @@ Create an HTML tree containing a normal eepp widget between/inside HTML elements
# Phase 2: Fix Confirmed Sibling Combinator Direction Bug
**Status: Implemented**
## Motivation
This is a confirmed correctness bug, not intentional behavior. Right-to-left selector matching for CSS combinators should follow CSS semantics:
@@ -1142,4 +1146,3 @@ Inlining is worthwhile but not the main algorithmic win. The main problem is can
- Keep string fields even after adding hashes to avoid breaking public APIs, debug output, CSS serialization, and tests.
- Prefer minimal changes with tests after each phase.
- Do not implement signature caching or sort redesign until instrumentation proves they are still needed.

View File

@@ -215,7 +215,7 @@ class EE_API Node : public Transformable {
virtual bool isType( const Uint32& type ) const;
/** @return True if this node is a UITextNode, false otherwise. */
bool isTextNode() const;
inline bool isTextNode() const { return 0 != ( mNodeFlags & NODE_FLAG_TEXTNODE ); }
/**
* @brief Posts a message to this node and its ancestors.
@@ -369,7 +369,7 @@ class EE_API Node : public Transformable {
*
* @return Pointer to the parent node or nullptr.
*/
Node* getParent() const;
inline Node* getParent() const { return mParentNode; }
/**
* @brief Sets the parent node.
@@ -429,7 +429,7 @@ class EE_API Node : public Transformable {
*
* @return Pointer to the next sibling node or nullptr.
*/
Node* getNextNode() const;
inline Node* getNextNode() const { return mNext; }
/**
* @brief Gets the previous sibling node in the parent's child list.
@@ -439,7 +439,7 @@ class EE_API Node : public Transformable {
*
* @return Pointer to the previous sibling node or nullptr.
*/
Node* getPrevNode() const;
inline Node* getPrevNode() const { return mPrev; }
/**
* @brief Gets the next sibling node, wrapping to first if at end.
@@ -525,7 +525,7 @@ class EE_API Node : public Transformable {
*
* @return The flags as a Uint32 bitmask.
*/
const Uint32& getNodeFlags() const;
inline const Uint32& getNodeFlags() const { return mNodeFlags; }
/**
* @brief Sets the node flags directly.
@@ -571,7 +571,7 @@ class EE_API Node : public Transformable {
*
* @return True if this node is a UIWidget, false otherwise.
*/
bool isWidget() const;
inline bool isWidget() const { return 0 != ( mNodeFlags & NODE_FLAG_WIDGET ); }
/**
* @brief Checks if this node is a Window.

View File

@@ -784,7 +784,11 @@ class EE_API UIWidget : public UINode {
*
* @return Pointer to the parent element widget or nullptr.
*/
UIWidget* getStyleSheetParentElement() const;
inline UIWidget* getStyleSheetParentElement() const {
return NULL != mParentNode && mParentNode->isWidget() && getType() != UI_TYPE_HTML_HTML
? mParentNode->asType<UIWidget>()
: NULL;
}
/**
* @brief Gets the previous sibling element for CSS styling.
@@ -793,7 +797,15 @@ class EE_API UIWidget : public UINode {
*
* @return Pointer to the previous sibling element widget or nullptr.
*/
UIWidget* getStyleSheetPreviousSiblingElement() const;
inline UIWidget* getStyleSheetPreviousSiblingElement() const {
Node* node = mPrev;
while ( NULL != node ) {
if ( node->isWidget() && !node->isTextNode() )
return node->asType<UIWidget>();
node = node->getPrevNode();
}
return NULL;
}
/**
* @brief Gets the next sibling element for CSS styling.
@@ -802,7 +814,15 @@ class EE_API UIWidget : public UINode {
*
* @return Pointer to the next sibling element widget or nullptr.
*/
UIWidget* getStyleSheetNextSiblingElement() const;
inline UIWidget* getStyleSheetNextSiblingElement() const {
Node* node = mNext;
while ( NULL != node ) {
if ( node->isWidget() && !node->isTextNode() )
return node->asType<UIWidget>();
node = node->getNextNode();
}
return NULL;
}
/**
* @brief Gets the active pseudo-classes for this widget.
@@ -811,7 +831,7 @@ class EE_API UIWidget : public UINode {
*
* @return Bitmask of active pseudo-classes.
*/
Uint32 getStyleSheetPseudoClasses() const { return mPseudoClasses; }
inline Uint32 getStyleSheetPseudoClasses() const { return mPseudoClasses; }
/**
* @brief Gets the pseudo-classes as string array.

View File

@@ -227,10 +227,6 @@ bool Node::isDisabled() const {
return !mEnabled;
}
Node* Node::getParent() const {
return mParentNode;
}
void Node::updateDrawInvalidator( bool force ) {
mNodeDrawInvalidator = getDrawInvalidator();
@@ -431,14 +427,6 @@ void Node::onClose() {
sendCommonEvent( Event::OnClose );
}
Node* Node::getNextNode() const {
return mNext;
}
Node* Node::getPrevNode() const {
return mPrev;
}
Node* Node::getNextNodeLoop() const {
if ( NULL == mNext )
return getParent()->getFirstChild();
@@ -527,10 +515,6 @@ Rectf Node::getLocalBounds() const {
return Rectf( 0, 0, mSize.getWidth(), mSize.getHeight() );
}
const Uint32& Node::getNodeFlags() const {
return mNodeFlags;
}
void Node::setNodeFlags( const Uint32& flags ) {
mNodeFlags = flags;
}
@@ -1094,14 +1078,6 @@ void Node::onSceneChange() {
}
}
bool Node::isWidget() const {
return 0 != ( mNodeFlags & NODE_FLAG_WIDGET );
}
bool Node::isTextNode() const {
return 0 != ( mNodeFlags & NODE_FLAG_TEXTNODE );
}
bool Node::isWindow() const {
return 0 != ( mNodeFlags & NODE_FLAG_WINDOW );
}

View File

@@ -189,7 +189,7 @@ bool StyleSheetSelector::select( UIWidget* element, const bool& applyPseudo ) co
break; // continue evaluating
}
case StyleSheetSelectorRule::PREVIOUS_SIBLING: {
curElement = curElement->getStyleSheetPreviousSiblingElement();
curElement = curElement->getStyleSheetNextSiblingElement();
if ( NULL == curElement || !selectorRule.matches( curElement, applyPseudo ) )
return false;
@@ -197,7 +197,7 @@ bool StyleSheetSelector::select( UIWidget* element, const bool& applyPseudo ) co
break; // continue evaluating
}
case StyleSheetSelectorRule::DIRECT_SIBLING: {
curElement = curElement->getStyleSheetNextSiblingElement();
curElement = curElement->getStyleSheetPreviousSiblingElement();
if ( NULL == curElement || !selectorRule.matches( curElement, applyPseudo ) )
return false;
@@ -206,24 +206,13 @@ bool StyleSheetSelector::select( UIWidget* element, const bool& applyPseudo ) co
}
case StyleSheetSelectorRule::SIBLING: {
bool foundSibling = false;
UIWidget* prevSibling = curElement->getStyleSheetPreviousSiblingElement();
UIWidget* nextSibling = curElement->getStyleSheetNextSiblingElement();
while ( NULL != prevSibling && !foundSibling ) {
if ( selectorRule.matches( prevSibling, applyPseudo ) ) {
for ( UIWidget* sibling = curElement->getStyleSheetPreviousSiblingElement();
NULL != sibling; sibling = sibling->getStyleSheetPreviousSiblingElement() ) {
if ( selectorRule.matches( sibling, applyPseudo ) ) {
curElement = sibling;
foundSibling = true;
} else {
prevSibling = prevSibling->getStyleSheetPreviousSiblingElement();
}
}
if ( !foundSibling ) {
while ( NULL != nextSibling && !foundSibling ) {
if ( selectorRule.matches( nextSibling, applyPseudo ) ) {
foundSibling = true;
} else {
nextSibling = nextSibling->getStyleSheetNextSiblingElement();
}
break;
}
}
@@ -321,26 +310,13 @@ std::vector<UIWidget*> StyleSheetSelector::getRelatedElements( UIWidget* element
}
case StyleSheetSelectorRule::SIBLING: {
bool foundSibling = false;
UIWidget* prevSibling = curElement->getStyleSheetPreviousSiblingElement();
UIWidget* nextSibling = curElement->getStyleSheetNextSiblingElement();
while ( NULL != prevSibling && !foundSibling ) {
if ( selectorRule.matches( prevSibling, applyPseudo ) ) {
for ( UIWidget* sibling = curElement->getStyleSheetPreviousSiblingElement();
NULL != sibling; sibling = sibling->getStyleSheetPreviousSiblingElement() ) {
if ( selectorRule.matches( sibling, applyPseudo ) ) {
foundSibling = true;
curElement = prevSibling;
} else {
prevSibling = prevSibling->getStyleSheetPreviousSiblingElement();
}
}
if ( !foundSibling ) {
while ( NULL != nextSibling && !foundSibling ) {
if ( selectorRule.matches( nextSibling, applyPseudo ) ) {
foundSibling = true;
curElement = nextSibling;
} else {
nextSibling = nextSibling->getStyleSheetNextSiblingElement();
}
curElement = sibling;
break;
}
}

View File

@@ -1050,32 +1050,6 @@ const std::string& UIWidget::getStyleSheetTag() const {
return mTag;
}
UIWidget* UIWidget::getStyleSheetParentElement() const {
return NULL != mParentNode && mParentNode->isWidget() && getType() != UI_TYPE_HTML_HTML
? mParentNode->asType<UIWidget>()
: NULL;
}
UIWidget* UIWidget::getStyleSheetPreviousSiblingElement() const {
Node* node = mPrev;
while ( NULL != node ) {
if ( node->isWidget() && !node->isTextNode() )
return node->asType<UIWidget>();
node = node->getPrevNode();
}
return NULL;
}
UIWidget* UIWidget::getStyleSheetNextSiblingElement() const {
Node* node = mNext;
while ( NULL != node ) {
if ( node->isWidget() && !node->isTextNode() )
return node->asType<UIWidget>();
node = node->getNextNode();
}
return NULL;
}
std::vector<const char*> UIWidget::getStyleSheetPseudoClassesStrings() const {
return StyleSheetSelectorRule::fromPseudoClass( mPseudoClasses );
}

View File

@@ -13,6 +13,7 @@
#include <eepp/system/filesystem.hpp>
#include <eepp/system/sys.hpp>
#include <eepp/ui/css/stylesheetparser.hpp>
#include <eepp/ui/css/stylesheetselector.hpp>
#include <eepp/ui/css/stylesheetspecification.hpp>
#include <eepp/ui/iconmanager.hpp>
#include <eepp/ui/tools/htmlformatter.hpp>
@@ -2695,6 +2696,113 @@ ul > li {
Engine::destroySingleton();
}
UTEST( UIHTML, StyleSheetTraversalBoundaries ) {
Engine::instance()->createWindow( WindowSettings( 1024, 768, "CSS Traversal Test",
WindowStyle::Default, WindowBackend::Default,
32, {}, 1, false, true ),
ContextSettings( false, 0, 0, GLv_default, true, false ) );
UISceneNode* sceneNode = init_test_inline_block();
const std::string html = R"html(
<!DOCTYPE html>
<html>
<head>
<style>
.goog-inline-block { display: inline-block; }
* html .goog-inline-block { display: inline; }
</style>
</head>
<body>
<a id="a" class="goog-inline-block"></a>
text
<div id="b"></div>
</body>
</html>
)html";
sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( html ) );
sceneNode->update( Seconds( 1 ) );
auto* htmlNode = sceneNode->getRoot()->findByType( UI_TYPE_HTML_HTML )->asType<UIWidget>();
auto* body = sceneNode->getRoot()->findByType( UI_TYPE_HTML_BODY )->asType<UIWidget>();
auto* a = sceneNode->getRoot()->find( "a" )->asType<UIHTMLWidget>();
auto* b = sceneNode->getRoot()->find( "b" )->asType<UIWidget>();
ASSERT_TRUE( htmlNode != nullptr );
ASSERT_TRUE( body != nullptr );
ASSERT_TRUE( a != nullptr );
ASSERT_TRUE( b != nullptr );
EXPECT_TRUE( htmlNode->getStyleSheetParentElement() == nullptr );
EXPECT_EQ( CSSDisplay::InlineBlock, a->getDisplay() );
EXPECT_TRUE( b->getStyleSheetPreviousSiblingElement() == a );
auto* nativeWidget = UIWidget::New();
nativeWidget->setParent( body );
auto* nativeChild = UIWidget::New();
nativeChild->setParent( nativeWidget );
EXPECT_TRUE( nativeWidget->getStyleSheetPreviousSiblingElement() == b );
EXPECT_TRUE( nativeChild->getStyleSheetParentElement() == nativeWidget );
Engine::destroySingleton();
}
UTEST( UIHTML, StyleSheetSiblingCombinators ) {
Engine::instance()->createWindow( WindowSettings( 1024, 768, "CSS Sibling Test",
WindowStyle::Default, WindowBackend::Default,
32, {}, 1, false, true ),
ContextSettings( false, 0, 0, GLv_default, true, false ) );
UISceneNode* sceneNode = init_test_inline_block();
const std::string html = R"html(
<!DOCTYPE html>
<html>
<head>
<style>
#a + #b { background-color: red; }
#b + #a { background-color: blue; }
#a ~ #c { background-color: green; }
#c ~ #a { background-color: blue; }
</style>
</head>
<body>
<div id="a"></div>
text
<div id="b"></div>
<span></span>
<div id="c"></div>
</body>
</html>
)html";
sceneNode->loadLayoutFromString( HTMLFormatter::HTMLtoXML( html ) );
sceneNode->update( Seconds( 1 ) );
auto* a = sceneNode->getRoot()->find( "a" )->asType<UIWidget>();
auto* b = sceneNode->getRoot()->find( "b" )->asType<UIWidget>();
auto* c = sceneNode->getRoot()->find( "c" )->asType<UIWidget>();
ASSERT_TRUE( a != nullptr );
ASSERT_TRUE( b != nullptr );
ASSERT_TRUE( c != nullptr );
EXPECT_TRUE( b->getBackgroundColor() == Color::Red );
EXPECT_TRUE( c->getBackgroundColor() == Color::Green );
EXPECT_TRUE( a->getBackgroundColor() != Color::Blue );
StyleSheetSelector directRelated( "#a:hover + #b" );
auto related = directRelated.getRelatedElements( b, false );
ASSERT_EQ( 1u, related.size() );
EXPECT_TRUE( related.front() == a );
StyleSheetSelector futureGeneralSibling( "#c:hover ~ #a" );
EXPECT_TRUE( futureGeneralSibling.getRelatedElements( a, false ).empty() );
StyleSheetSelector inverseSibling( "#b | #a" );
EXPECT_TRUE( inverseSibling.select( a ) );
EXPECT_FALSE( inverseSibling.select( b ) );
Engine::destroySingleton();
}
UTEST( UIHTML, BlockList ) {
Engine::instance()->createWindow( WindowSettings( 1024, 768, "Block List Test",
WindowStyle::Default, WindowBackend::Default,