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

Avoid style recalculation in breathe behavior #3919

Closed
bhousel opened this issue Mar 21, 2017 · 7 comments
Closed

Avoid style recalculation in breathe behavior #3919

bhousel opened this issue Mar 21, 2017 · 7 comments
Labels
performance Optimizing for speed and efficiency

Comments

@bhousel
Copy link
Member

bhousel commented Mar 21, 2017

Currently d3 is doing a getPropertyValue to determine the original line width / circle radius in order to determine the animation parameters for the breathing. We should just hardcode some reasonable values for vertices and lines to avoid doing this.

screenshot 2017-03-21 13 52 49

@bhousel bhousel added performance Optimizing for speed and efficiency priority A top priority issue that has a big impact or matter most to new mappers labels Mar 21, 2017
@bhousel bhousel added this to Other Priorities in Priorities Jul 6, 2017
@bhousel
Copy link
Member Author

bhousel commented Jul 11, 2017

A good read on this topic
https://medium.com/outsystems-experts/how-to-achieve-60-fps-animations-with-css3-db7b98610108

It applies more broadly to performance than just this issue.

@kepta
Copy link
Collaborator

kepta commented Aug 1, 2017

image
I got a chance to see forced reflow on chrome v60.

@kepta
Copy link
Collaborator

kepta commented Aug 1, 2017

@bhousel this might sound weird, but how about we disable animation and keep it simple.

@tyrasd
Copy link
Member

tyrasd commented Aug 1, 2017

related comment: #2911 (comment)

@bhousel
Copy link
Member Author

bhousel commented Aug 1, 2017

I'd really like to keep the animation. Before we had it, we got a lot of feedback from people saying they couldn't see when things were selected. (colorblind people especially need this)

I think this animation is pretty simple and we should be able to do it without triggering reflow. Here are some more thoughts:

  • opacity/transparency interpolation

    • browsers should have this optimized
    • can hardcode min and max values, no need to query style at each animation cycle
    • can possibly just interpolate the opacity of whatever layer all these shadow strokes are on, rather than interpolating the opacity of each shadow stroke
  • vertices (circles)

    • we currently interpolate radius, but we might be more performant in browsers to interpolate scale of the circle
    • we query min and max radius values, as there are a few different sized vertices (endpoints or vertices with icon are larger)
    • if we switch to interpolating scale instead of radius, can hardcode min/max like 0.8/1.2 or something and avoid the style query
  • nodes (poi markers)

    • all poi markers are the same, so we can hardcode min/max stroke-width for this too, or even better interpolate scale instead of stroke-width.
  • ways

    • this is were we most need to query the stroke-width and calculate a min/max range because
      • there are several different stroke widths of ways (think area edges, thin highways, thick highways, airport runways, etc)
      • toggling wireframe mode while things are selected will change the stroke-width too
  • another thing that seems very slow is how we are adding the interpolator behavior to each thing selected. Use the lasso to select 20 or more vertices and you will see it slow down a lot. (though vertices are circles and there may be low hanging fruit to improve the perf of those, described above)

@bhousel bhousel added the wip Work in progress label Aug 9, 2017
@bhousel
Copy link
Member Author

bhousel commented Aug 9, 2017

wip at #4177

@bhousel bhousel removed the priority A top priority issue that has a big impact or matter most to new mappers label Oct 1, 2017
@bhousel bhousel removed this from Other Priorities in Priorities Oct 3, 2017
@bhousel
Copy link
Member Author

bhousel commented Jun 24, 2018

stale

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

No branches or pull requests

3 participants