From bb6da721fbfbf4111254d69d82164b4b93b8ad66 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Sat, 4 Jul 2026 20:22:44 +0200 Subject: [PATCH] Index style rules by their rightmost simple selector computeStyle tested every selector alternative of every rule against every element. RuleIndex buckets each alternative by the id, class, or element type its rightmost compound requires; alternatives without such a key (universal, attribute-only or pseudo-only compounds) go into a remainder bucket consulted for every element. A lookup gathers the buckets the element satisfies and hands the candidates to the matcher in cascade order, so match positions and cascade results are unchanged. RuleIndexTest locks the completeness invariant: over a stylesheet covering every selector shape, the index never drops an alternative the matcher would accept. --- .../css/core/impl/engine/CSSEngineImpl.java | 53 +++-- .../core/impl/engine/selector/RuleIndex.java | 186 ++++++++++++++++++ .../e4/ui/tests/css/core/RuleIndexTest.java | 159 +++++++++++++++ .../ui/tests/css/core/parser/ViewCSSTest.java | 31 ++- 4 files changed, 382 insertions(+), 47 deletions(-) create mode 100644 bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/selector/RuleIndex.java create mode 100644 tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/RuleIndexTest.java diff --git a/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/CSSEngineImpl.java b/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/CSSEngineImpl.java index bb6c95d000f..6b66348d1e5 100644 --- a/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/CSSEngineImpl.java +++ b/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/CSSEngineImpl.java @@ -61,6 +61,7 @@ import org.eclipse.e4.ui.css.core.impl.dom.CSSStyleSheetImpl; import org.eclipse.e4.ui.css.core.impl.dom.CssRule; import org.eclipse.e4.ui.css.core.impl.dom.StyleWrapper; +import org.eclipse.e4.ui.css.core.impl.engine.selector.RuleIndex; import org.eclipse.e4.ui.css.core.impl.engine.selector.SelectorMatcher; import org.eclipse.e4.ui.css.core.impl.engine.selector.Selectors; import org.eclipse.e4.ui.css.core.resources.IResourcesRegistry; @@ -102,8 +103,8 @@ public abstract class CSSEngineImpl implements CSSEngine { /** The stylesheets added to this engine, in registration order. */ private final List styleSheets = new ArrayList<>(); - /** Cached flat list of style rules over all stylesheets. */ - private List combinedRules; + /** Cached rule index over all stylesheets. */ + private RuleIndex ruleIndex; /** * {@link IElementProvider} used to retrieve w3c Element linked to the @@ -224,7 +225,7 @@ private CSSStyleSheetImpl parseStyleSheet(String content, String uri) throws IOE /** Register a parsed stylesheet with this engine's cascade. */ public void addStyleSheet(CSSStyleSheetImpl styleSheet) { styleSheets.add(styleSheet); - combinedRules = null; + ruleIndex = null; } /** The stylesheets registered with this engine, in registration order. */ @@ -974,22 +975,20 @@ public CSSStyleDeclaration computeStyle(Element elt, String pseudoElt) { hierarchy[idx++] = (Element) n; } - for (CSSStyleRuleImpl rule : getCombinedRules()) { - for (Selectors.Selector selector : rule.getSelectorList().alternatives()) { - if (SelectorMatcher.matches(selector, elt, pseudoElt, hierarchy, 0)) { - int specificity = selector.specificity(); - StyleWrapper wrapper = new StyleWrapper(rule.getStyle(), specificity, position++); - if (firstStyleDeclaration == null) { - firstStyleDeclaration = wrapper; - } else { - // There are several style declarations which match - // the current element - if (styleDeclarations == null) { - styleDeclarations = new ArrayList<>(); - styleDeclarations.add(firstStyleDeclaration); - } - styleDeclarations.add(wrapper); + for (RuleIndex.Candidate candidate : getRuleIndex().candidatesFor(elt)) { + if (SelectorMatcher.matches(candidate.selector(), elt, pseudoElt, hierarchy, 0)) { + int specificity = candidate.selector().specificity(); + StyleWrapper wrapper = new StyleWrapper(candidate.rule().getStyle(), specificity, position++); + if (firstStyleDeclaration == null) { + firstStyleDeclaration = wrapper; + } else { + // There are several style declarations which match + // the current element + if (styleDeclarations == null) { + styleDeclarations = new ArrayList<>(); + styleDeclarations.add(firstStyleDeclaration); } + styleDeclarations.add(wrapper); } } } @@ -1004,19 +1003,11 @@ public CSSStyleDeclaration computeStyle(Element elt, String pseudoElt) { return null; } - private List getCombinedRules() { - if (combinedRules == null) { - List rules = new ArrayList<>(); - for (CSSStyleSheetImpl styleSheet : styleSheets) { - for (CssRule rule : styleSheet.getRules()) { - if (rule instanceof CSSStyleRuleImpl styleRule) { - rules.add(styleRule); - } - } - } - combinedRules = rules; + private RuleIndex getRuleIndex() { + if (ruleIndex == null) { + ruleIndex = RuleIndex.of(styleSheets); } - return combinedRules; + return ruleIndex; } @Override @@ -1041,7 +1032,7 @@ public void dispose() { @Override public void reset() { styleSheets.clear(); - combinedRules = null; + ruleIndex = null; } /*--------------- Resources Registry -----------------*/ diff --git a/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/selector/RuleIndex.java b/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/selector/RuleIndex.java new file mode 100644 index 00000000000..9bc2431f7e0 --- /dev/null +++ b/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/selector/RuleIndex.java @@ -0,0 +1,186 @@ +/******************************************************************************* + * Copyright (c) 2026 Lars Vogel and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Lars Vogel - initial API and implementation + *******************************************************************************/ +package org.eclipse.e4.ui.css.core.impl.engine.selector; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.eclipse.e4.ui.css.core.dom.CSSStylableElement; +import org.eclipse.e4.ui.css.core.impl.dom.CSSStyleRuleImpl; +import org.eclipse.e4.ui.css.core.impl.dom.CSSStyleSheetImpl; +import org.eclipse.e4.ui.css.core.impl.dom.CssRule; +import org.eclipse.e4.ui.css.core.impl.engine.selector.Selectors.Selector; +import org.w3c.dom.Element; + +/** + * Buckets style rules by the rightmost simple selector of each selector + * alternative, so the cascade only runs the matcher against rules that can + * possibly match a given element. + */ +public final class RuleIndex { + + /** One selector alternative of a style rule, with its cascade position. */ + public record Candidate(Selector selector, CSSStyleRuleImpl rule, int order) { + } + + private static final Comparator BY_ORDER = Comparator.comparingInt(Candidate::order); + + private final Map> byId = new HashMap<>(); + private final Map> byClass = new HashMap<>(); + private final Map> byTag = new HashMap<>(); + private final List remainder = new ArrayList<>(); + private final List all = new ArrayList<>(); + + private RuleIndex() { + } + + /** Indexes every style rule of the given stylesheets in cascade order. */ + public static RuleIndex of(List styleSheets) { + RuleIndex index = new RuleIndex(); + for (CSSStyleSheetImpl styleSheet : styleSheets) { + for (CssRule rule : styleSheet.getRules()) { + if (rule instanceof CSSStyleRuleImpl styleRule) { + for (Selector alternative : styleRule.getSelectorList().alternatives()) { + index.add(new Candidate(alternative, styleRule, index.all.size())); + } + } + } + } + return index; + } + + private void add(Candidate candidate) { + all.add(candidate); + Selector key = rightmostKey(candidate.selector()); + if (key instanceof Selectors.IdSelector id) { + byId.computeIfAbsent(id.id(), k -> new ArrayList<>()).add(candidate); + } else if (key instanceof Selectors.ClassSelector cls) { + byClass.computeIfAbsent(cls.className(), k -> new ArrayList<>()).add(candidate); + } else if (key instanceof Selectors.ElementType type) { + byTag.computeIfAbsent(type.localName(), k -> new ArrayList<>()).add(candidate); + } else { + remainder.add(candidate); + } + } + + /** + * The most selective simple selector the rightmost compound requires of + * the subject element: id over class over element type. {@code null} + * when the compound only constrains the element by attributes or pseudo + * classes, or not at all; such alternatives go into the remainder bucket + * consulted for every element. + */ + private static Selector rightmostKey(Selector selector) { + if (selector instanceof Selectors.Descendant d) { + return rightmostKey(d.descendant()); + } + if (selector instanceof Selectors.Child c) { + return rightmostKey(c.child()); + } + if (selector instanceof Selectors.Adjacent a) { + return rightmostKey(a.second()); + } + if (selector instanceof Selectors.And and) { + Selector left = rightmostKey(and.left()); + Selector right = rightmostKey(and.right()); + return rank(left) >= rank(right) ? left : right; + } + if (selector instanceof Selectors.IdSelector || selector instanceof Selectors.ClassSelector) { + return selector; + } + if (selector instanceof Selectors.ElementType type && type.localName() != null) { + return selector; + } + return null; + } + + private static int rank(Selector key) { + if (key instanceof Selectors.IdSelector) { + return 3; + } + if (key instanceof Selectors.ClassSelector) { + return 2; + } + if (key instanceof Selectors.ElementType) { + return 1; + } + return 0; + } + + /** + * The candidates whose bucket key the element satisfies, in cascade + * order. A superset of the actually matching alternatives; callers still + * run the matcher on each candidate. + */ + public List candidatesFor(Element element) { + List result = new ArrayList<>(remainder); + String elementName = element.getPrefix() == null ? element.getNodeName() : element.getLocalName(); + addBucket(byTag, elementName, result); + if (element instanceof CSSStylableElement stylable) { + addClassBuckets(stylable.getCSSClass(), result); + addBucket(byId, stylable.getCSSId(), result); + } + result.sort(BY_ORDER); + return result; + } + + /** Every indexed candidate, in cascade order. */ + public List allCandidates() { + return Collections.unmodifiableList(all); + } + + private static void addBucket(Map> buckets, String key, List result) { + if (key == null || buckets.isEmpty()) { + return; + } + List bucket = buckets.get(key); + if (bucket != null) { + result.addAll(bucket); + } + } + + private void addClassBuckets(String cssClass, List result) { + if (cssClass == null || byClass.isEmpty()) { + return; + } + // Same whitespace-token semantics as the matcher's class comparison; + // a duplicate word must not add its bucket twice. + List words = null; + int length = cssClass.length(); + int i = 0; + while (i < length) { + while (i < length && Character.isWhitespace(cssClass.charAt(i))) { + i++; + } + int start = i; + while (i < length && !Character.isWhitespace(cssClass.charAt(i))) { + i++; + } + if (i > start) { + String word = cssClass.substring(start, i); + if (words == null) { + words = new ArrayList<>(2); + } + if (!words.contains(word)) { + words.add(word); + addBucket(byClass, word, result); + } + } + } + } +} diff --git a/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/RuleIndexTest.java b/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/RuleIndexTest.java new file mode 100644 index 00000000000..d9aef9f4dd8 --- /dev/null +++ b/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/RuleIndexTest.java @@ -0,0 +1,159 @@ +/******************************************************************************* + * Copyright (c) 2026 Lars Vogel and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Lars Vogel - initial API and implementation + *******************************************************************************/ +package org.eclipse.e4.ui.tests.css.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.StringReader; +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.e4.ui.css.core.engine.CSSEngine; +import org.eclipse.e4.ui.css.core.impl.engine.selector.RuleIndex; +import org.eclipse.e4.ui.css.core.impl.engine.selector.SelectorMatcher; +import org.eclipse.e4.ui.tests.css.core.util.ParserTestUtil; +import org.eclipse.e4.ui.tests.css.core.util.TestElement; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.w3c.dom.css.CSSStyleDeclaration; + +/** + * Verifies that the rule index never filters out an alternative the matcher + * would accept and that candidates keep cascade order. + */ +public class RuleIndexTest { + + private static final String SHEET = """ + Button { p1: v; } + .warning { p2: v; } + #go { p3: v; } + Button.warning { p4: v; } + Button#go.warning { p5: v; } + Composite Button { p6: v; } + Shell > .warning { p7: v; } + Label + Button { p8: v; } + * { p9: v; } + [role] { p10: v; } + [role='editor'] { p11: v; } + [tags~='x'] { p12: v; } + :selected { p13: v; } + Button:selected { p14: v; } + Button, .warning, #go, [role], Text { p15: v; } + Shell Composite Button.warning { p16: v; } + Composite > Button#go { p17: v; } + .other.warning { p18: v; } + """; + + private CSSEngine engine; + private RuleIndex index; + + @BeforeEach + void setUp() throws Exception { + engine = ParserTestUtil.createEngine(); + index = RuleIndex.of(List.of(ParserTestUtil.parseCss(SHEET))); + } + + private List buildElements() { + List elements = new ArrayList<>(); + TestElement shell = new TestElement("Shell", engine); + elements.add(shell); + TestElement composite = new TestElement("Composite", shell, engine); + elements.add(composite); + + TestElement button = new TestElement("Button", composite, engine); + button.setClass("warning other"); + button.setId("go"); + button.setAttribute("role", "editor"); + button.setAttribute("tags", "x y"); + elements.add(button); + + TestElement label = new TestElement("Label", composite, engine); + elements.add(label); + TestElement adjacentButton = new TestElement("Button", composite, engine); + elements.add(adjacentButton); + + TestElement duplicateClasses = new TestElement("Text", shell, engine); + duplicateClasses.setClass("warning warning"); + elements.add(duplicateClasses); + + TestElement other = new TestElement("Text", shell, engine); + other.setClass("other"); + other.setAttribute("role", "outline"); + elements.add(other); + + TestElement plain = new TestElement("Canvas", shell, engine); + elements.add(plain); + return elements; + } + + @Test + void testCandidatesCompleteAndOrdered() { + for (TestElement element : buildElements()) { + assertCandidatesComplete(element, null); + assertCandidatesComplete(element, "selected"); + } + } + + private void assertCandidatesComplete(TestElement element, String pseudo) { + List linearMatches = new ArrayList<>(); + for (RuleIndex.Candidate candidate : index.allCandidates()) { + if (SelectorMatcher.matches(candidate.selector(), element, pseudo)) { + linearMatches.add(candidate.order()); + } + } + List indexedMatches = new ArrayList<>(); + int previousOrder = -1; + for (RuleIndex.Candidate candidate : index.candidatesFor(element)) { + assertTrue(candidate.order() > previousOrder, + "candidates for " + element.getLocalName() + " not in cascade order"); + previousOrder = candidate.order(); + if (SelectorMatcher.matches(candidate.selector(), element, pseudo)) { + indexedMatches.add(candidate.order()); + } + } + assertEquals(linearMatches, indexedMatches, + "index dropped matching alternatives for " + element.getLocalName()); + } + + @Test + void testCandidatesArePrunedForPlainElement() { + TestElement plain = new TestElement("Canvas", engine); + List candidates = index.candidatesFor(plain); + assertFalse(candidates.isEmpty()); + assertTrue(candidates.size() < index.allCandidates().size(), + "expected the index to prune non-applicable rules"); + } + + @Test + void testComputedStyleMergesAcrossBuckets() throws Exception { + engine.parseStyleSheet(new StringReader( + "Button { color: red; } .warning { color: blue; }")); + TestElement button = new TestElement("Button", engine); + button.setClass("warning"); + CSSStyleDeclaration style = engine.computeStyle(button, null); + assertEquals("blue", style.getPropertyCSSValue("color").getCssText()); + } + + @Test + void testComputedStyleKeepsDocumentOrderOnEqualSpecificity() throws Exception { + engine.parseStyleSheet(new StringReader( + ".a { color: red; } .b { color: green; }")); + TestElement element = new TestElement("Button", engine); + element.setClass("a b"); + CSSStyleDeclaration style = engine.computeStyle(element, null); + assertEquals("green", style.getPropertyCSSValue("color").getCssText()); + } +} diff --git a/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/parser/ViewCSSTest.java b/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/parser/ViewCSSTest.java index d34b2614f1e..ea81b95d591 100644 --- a/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/parser/ViewCSSTest.java +++ b/tests/org.eclipse.e4.ui.tests.css.core/src/org/eclipse/e4/ui/tests/css/core/parser/ViewCSSTest.java @@ -25,9 +25,9 @@ import java.io.StringReader; import java.lang.reflect.Field; import java.lang.reflect.Method; -import java.util.List; import org.eclipse.e4.ui.css.core.impl.engine.CSSEngineImpl; +import org.eclipse.e4.ui.css.core.impl.engine.selector.RuleIndex; import org.eclipse.e4.ui.css.swt.engine.CSSSWTEngineImpl; import org.eclipse.e4.ui.tests.css.core.util.TestElement; import org.eclipse.swt.widgets.Display; @@ -120,16 +120,15 @@ void testBug419482_higherSpecificity() throws Exception { assertEquals("color: blue;", buttonStyle.getCssText()); } - @SuppressWarnings("unchecked") @Test void testRuleCaching() throws Exception { String css = "Shell > * > * { color: red; }\n" + "Button { color: blue; }\n"; parseStyleSheet(css); - Field combinedRulesField = CSSEngineImpl.class.getDeclaredField("combinedRules"); - combinedRulesField.setAccessible(true); + Field ruleIndexField = CSSEngineImpl.class.getDeclaredField("ruleIndex"); + ruleIndexField.setAccessible(true); // before the first computeStyle() call the cache is empty - assertNull(combinedRulesField.get(engine)); + assertNull(ruleIndexField.get(engine)); final TestElement shell = new TestElement("Shell", engine); final TestElement composite = new TestElement("Composite", shell, engine); @@ -138,25 +137,25 @@ void testRuleCaching() throws Exception { assertNotNull(buttonStyle); // now the cache is filled - assertNotNull(combinedRulesField.get(engine)); + assertNotNull(ruleIndexField.get(engine)); - // deeper inspection: check what private method getCombinedRules returns - Method getCombinedRulesMethod = CSSEngineImpl.class.getDeclaredMethod("getCombinedRules"); - getCombinedRulesMethod.setAccessible(true); - List cssRules = (List) getCombinedRulesMethod.invoke(engine); + // deeper inspection: check what private method getRuleIndex returns + Method getRuleIndexMethod = CSSEngineImpl.class.getDeclaredMethod("getRuleIndex"); + getRuleIndexMethod.setAccessible(true); + RuleIndex index = (RuleIndex) getRuleIndexMethod.invoke(engine); - // check caching: a 2nd call retrieves the cached list - assertSame(cssRules, getCombinedRulesMethod.invoke(engine)); + // check caching: a 2nd call retrieves the cached index + assertSame(index, getRuleIndexMethod.invoke(engine)); // add a new stylesheet => flush cache css = "Shell > * > * { color: blue; }\n" + "Label { color: green; }\n"; parseStyleSheet(css); - assertNull(combinedRulesField.get(engine)); + assertNull(ruleIndexField.get(engine)); - List cssRules2 = (List) getCombinedRulesMethod.invoke(engine); - assertNotSame(cssRules, cssRules2); + RuleIndex index2 = (RuleIndex) getRuleIndexMethod.invoke(engine); + assertNotSame(index, index2); // stylesheet added => more rules - assertTrue(cssRules2.size() > cssRules.size()); + assertTrue(index2.allCandidates().size() > index.allCandidates().size()); } private void parseStyleSheet(String css) throws IOException {