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

try some placeholder values for breathe.js #4177

Closed
wants to merge 1 commit into from
Closed

try some placeholder values for breathe.js #4177

wants to merge 1 commit into from

Conversation

kepta
Copy link
Collaborator

@kepta kepta commented Jul 24, 2017

I tried to use some reasonable values for the fills and radius.

I guess we want a bit more fine grained placeholder values?

ref: #3919

@bhousel
Copy link
Member

bhousel commented Jul 25, 2017

I tested with the Chrome 59 profiler to see if these changes make a difference but it's not really clear to me. I think there is a benefit but I will need to do some more testing when I'm more awake. Also, Chrome changed their profiler around 58 so I'm not super familiar with the new Performance tab.

One thing for sure, the max line width will need to be wider. It is obscured on thick roads.

The reason we used s.style before (to get the calculated style) was because:

  • When the user changes a line preset, it may have a new width . e.g. changing from a think line highway=residential to a thin line highway=service, or power=line
  • When the user toggles wireframe everything has small width and needs to change.

@bhousel
Copy link
Member

bhousel commented Jul 30, 2017

Hey @kepta, last night Chrome 60 installed itself, so I took a look with the new profiler and I'm not sure whether querying the style is causing frame drops anymore.

Let's chat this week about performance stuff. There is a LOT in here that I don't understand and it would be great for us both to learn more about it.

@bhousel bhousel added the waitfor Waiting for something label Jul 30, 2017
@kepta
Copy link
Collaborator Author

kepta commented Jul 31, 2017

@bhousel sounds great.

@bhousel
Copy link
Member

bhousel commented Jun 29, 2018

closing as stale

@bhousel bhousel closed this Jun 29, 2018
@bhousel bhousel deleted the perf branch November 3, 2018 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waitfor Waiting for something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants