search

MLH-Fellowship / scheduling-profiler-prototype

Custom profiler prototype for React's concurrent mode
https://react-scheduling-profiler.vercel.app/
6 stars 0 forks source link

[Optimize flamechart][4/4] Split FlamechartView into row views #93

Closed taneliang closed 4 years ago

taneliang commented 4 years ago

Stack PR by STACK ATTACK:

Summary

Allow the view system to render only affected flamechart rows. This greatly improves flamechart hover performance, which has a high impact on our app's performance as the flamechart is the slowest view to render.

Also adds a new ColorView view that fills the flamechart area with a solid background color to fix these issues:

  1. Black empty areas appearing below flamecharts without much stack depth
  2. Lines appearing between flamechart rows. My hypothesis is that these are appearing due to subpixel rendering, even though the flamechart rows' visibleAreas are all integers.

Related to #50.

Test Plan

vercel[bot] commented 4 years ago

This pull request is being automatically deployed with Vercel (learn more). To see the status of your deployment, click below or on the icon next to each commit.

šŸ” Inspect: https://vercel.com/mlh-fellowship/scheduling-profiler-prototype/cx4f68a6z āœ… Preview: https://scheduling-profiler-prototype-git-0f8a7-4.mlh-fellowship.vercel.app

taneliang commented 4 years ago

Holding off on merging this due to these 2 known issues:

  1. Lines appear between flamechart stack layer rows when the browser is zoomed in (https://github.com/MLH-Fellowship/scheduling-profiler-prototype/pull/96#issuecomment-665473507) image

  2. Flamechart view no longer expands to fill all available space, leaving a black area at the bottom. image

kartikcho commented 4 years ago

Update: The issue is still observable when zoomed out.

image

taneliang commented 4 years ago

I can't repro this when zoomed out. Is your screen a hidpi screen?

kartikcho commented 4 years ago

I can't repro this when zoomed out. Is your screen a hidpi screen?

Yes but that shouldn't be the issue here I think.

taneliang commented 4 years ago

that shouldn't be the issue here I think.

You may be right. Without zooming in, I can't repro this on my Mac that has a HiDPI screen either.

taneliang commented 4 years ago

I'll fix this tomorrow

taneliang commented 4 years ago

I'll fix this tomorrow

I lied, this should be fixed. @kartikcho could you see if you still see this issue?

taneliang commented 4 years ago

Okay nevermind I caused a zooming bug. I'll fix this tomorrow.

image

taneliang commented 4 years ago

I'll fix this tomorrow.

I lied again, it's fixed šŸ˜† I accidentally wrote .x instead of .y. @kartikcho could you check again if you can still repro the issue you brought up?

kartikcho commented 4 years ago

Seems fixed as I can't repro it anymore, good job!