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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detecting invalidation cases efficiently #17

Open
hrj opened this issue Feb 28, 2015 · 7 comments
Open

Detecting invalidation cases efficiently #17

hrj opened this issue Feb 28, 2015 · 7 comments

Comments

@hrj
Copy link
Contributor

hrj commented Feb 28, 2015

Use-case

In the browser, when the mouse is hovered over an element, we need to find out whether that element has a hover style and then invalidate the layout+renderer accordingly.

Question

Is there a good / optimal way to do this? If not, can it be added?

Suggestion

One way could be: the analyser has an API that returns a list of applicable rules for an element. The caller can then walk the rules to see if any rule depends on a specified pseudo-element and hence requires invalidation.

@radkovo
Copy link
Owner

radkovo commented Mar 5, 2015

Hi, I am sorry for the delay. I have some deadlines in other projects this month so I am not able to fully focus on this issue now. But I think your suggestion is very good. For each element, we could maintain some list of pseudo classes that may influence its style. Then, we could decide which elements need to be recomputed. If you have some more specific solution in mind, feel free to add the necessary support to the existing functions.

@hrj
Copy link
Contributor Author

hrj commented Mar 6, 2015

Thanks for the reply @radkovo. I will try to come up with some code for this. Here's my plan:

The current API for getting an element's style is: DirectAnalyser.getElementStyle(element, pseudoDeclaration, mediaQuery).

This computes everything and returns the NodeData for the element.

I propose creating smaller, more atomic APIs:

1️⃣ DirectAnalyser.getElementApplicableRules(element, pseudoDeclaration, mediaQuery)
2️⃣ DirectAnalyser.getElementStyle(applicableRules, element)
3️⃣ DirectAnalyser.hasPseudoStyle(element, applicableRules, ancestor, pseudoDeclaration)

The current .getElementStyle() could be implemented using 1️⃣ and 2️⃣.

3️⃣ will be a new API that returns true if an element's style is affected by a pseduoDeclaration. I will explain the ancestor parameter shortly.

This kind of separation will help optimise multiple cases in a browser:

When a element is hovered over by mouse or when its attributes change, there is no need to find the applicable rules again; the previously computed list can be cached and passed to 2️⃣

Now, about the ancestor parameter in 3️⃣. It is a little complex to explain. Imagine the following html:

<div id="gp">
    Grand Parent
    <div id="parent">
         Parent
        <div id="child">Child</div>
    </div>
</div>

And the following CSS

#gp:hover #child {background: red }

That is the child's background should be red when mouse is hovered over #gp. To analyse such scenarios, the ancestor element is useful. When mouse enters #gp element, the call to 3️⃣ will check whether :hover style is affected for #child, with ancestor = #gp element.


Please let me know if you have any feedback. I will begin implementation if this makes sense.

@radkovo
Copy link
Owner

radkovo commented Mar 8, 2015

Hi, it makes perfect sense to me, I agree. Just one thing: in the last example, how do we determine which ancestor to use? E.g. when mouse enters #gp and #parent simultaneously, which one do you use as the ancestor?

@hrj
Copy link
Contributor Author

hrj commented Mar 9, 2015

when mouse enters #gp and #parent simultaneously, which one do you use as the ancestor?

Glad you asked; I hadn't considered that scenario!

I imagine that it would result in two calls, one with #gp as ancestor and the other with #parent. Alternatively, the API could accept a list of ancestors. That choice probably depends on how the caller is structured. (In the current design of gngr, making multiple calls is a natural fit but accepting a list of ancestors is a better / more general solution).

Does that make sense? and do you have a preference for it?

@radkovo
Copy link
Owner

radkovo commented Mar 11, 2015

That seems reasonable. I have no strong preference about it but I mean separate calls are general enough and it's not necessary to setup a collection for making the call. I was thinking about another alternative: implementing a method like

List<Element> DirectAnalyser.getPseudoAncestors(element, pseudoDeclaration)

that would return all the ancestors affected by the given pseudo declaration. Or simply just without specifying the pseudo (i.e. considering any pseudo) and you could do the filtering later. It would return an empty list when the element style does not depend on any pseudo. However, I haven't not thought about how expensive it would be to maintain such a list for every element. What do you think?

@hrj
Copy link
Contributor Author

hrj commented Jul 5, 2015

@radkovo Sorry for the late reply. Hadn't found time to think deeply about this, until now.

I am not sure if the API you proposed (getPseudoAncestors) would be beneficial for the general case. In general, the changes in styles (triggered by changes in DOM) propagate down the hierarchy. (Cascade down). So it is easier to update from the ancestor down to the descendants. To make full use of your API, one would need to update all the descendants whenever a DOM change happens, and then check if any ancestors match. (There might be more sophisticated ways of updating only a subset of descendants; I have not thought beyond simple algorithms).

However, your API might be suitable for the relational selector where changes can flow from descendants to ancestors. I personally think that the relational selector will be very useful. But it is still a draft as of now.

@hrj
Copy link
Contributor Author

hrj commented Jul 6, 2015

@radkovo Ignore my previous comment. It was rather pointless. The API you have proposed is fine; except the memory requirement would be high (as you rightly pointed out). Perhaps, the ancestor data can be cached for only a part of the DOM tree (for example, in the case of :hover, the part underneath the current cursor position). Even then, there will be a small overhead of reserving a pointer in the element, and the cost of ensuring it is invalidated whenever the DOM changes.

In the first cut, I will try to implement a non-caching approach (the one I proposed initially). I have implemented the getElementApplicableRules today. I am working on the hasPseudoStyle API now.

hrj added a commit to UprootLabs/jStyleParser that referenced this issue Jul 7, 2015
* Refactored some of the previous implementations into a pure static
class, keeping the existing API intact.

s* mall optimisation: use array instead of array list
@hrj hrj mentioned this issue Jul 7, 2015
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

No branches or pull requests

2 participants