Reimplement canvas using view classes

[taneliang]

Summary

TL;DR

Replaces Brian's original procedural code with view classes. The views are rudimentary implementations of iOS UIKit-style UIViews.

Originally discussed at #50 (comment).

Related to #50. Resolves #71, resolves #39, resolves #83. Unblocks others (#70, #72)

This PR is enormous but I can't really split it, as I'm fixing conceptual issues as I go.

Problems this solves

• The current code calculates the same layout in 2 places (renderCanvas and getHoveredEvent) as we aren't able to share layout code between them.
• Pan and zoom state is currently stored globally by usePanAndZoom. This is a problem as Add vertical resizer between canvas #19 requires resizing and independent scrolling of 2 areas of the same canvas (or 2 separate canvasses). This view architecture will allow us to implement independent scrolling within 1 canvas by storing state in individual classes.
• The whole canvas is redrawn on every state change in the CanvasPage React component, which in practice meant that moving the cursor across the canvas triggered a full-canvas render. This view architecture solves this with:
  • State that CanvasPage does not need will be stored in mutable refs or as variables in the view instances.
  • Views are only redrawn if setNeedsDisplay or setSubviewsNeedDisplay has been called on them. If a view needs display, it'll lay out its subviews and draw them. This allows us to easily redraw parts of the canvas that have changed, improving hovering performance. Note that this does not improve scroll/zoom operations that require a near-full-canvas redraw.
  • Interactions don't go through React at all. Custom views can be initialized callbacks if CanvasPage state needs to be modified.

Pros

• Using a view hierarchy allows us to more easily play with the layout of our views.
• View classes allow us to encapsulate a view's logic (specifically drawing and interactions) in one place.
• Enables quicker iterations by allowing us to define content view types easily.
• Layout code is also now reusable and separated from content views.
• Performant by default - offscreen views are automatically not drawn, and views with no updates are also not drawn.

Cons

• It is somewhat complicated and is another thing to learn, especially if you don't have an iOS background.

Classes

All views inherit from the View class. The main classes and key purposes are listed below:

• Surface: Represents the canvas surface. Also the main point of entry for all incoming interactions.
• View: Base class that can be subclassed to implement custom content views (e.g. our flamegraph, React data, etc.). This is the equivalent of UIKit's UIView.
• StaticLayoutView: A simple View subclass that can display a list of subviews laid out by an injected layouter function. This is used for horizontal and vertical stacking as well as layering.
• HorizontalPanAndZoomView: A View subclass that handles horizontal pan, scroll and zoom interactions. This contains a single contentView subview. When panning and zooming, the contentView's frame is mutated. Known issue: the contentView's frame is not clamped and you can scroll offscreen.

More envisioned layout views:

• VerticalScrollView: A View subclass that handles vertical pan and scroll interactions. This will be similar to HorizontalPanAndZoomView except that it does not zoom.
• ResizableSplitView: A View subclass that has 2 subviews divided by a resize bar. For (Add vertical resizer between canvas #19).

This design is not the cleanest as we complect a bunch of behaviors into the same class, and it's strange that only one class has an injected layouter. This can be cleaned up in the future if needed.

Content views:

• TimeAxisMarkersView
• ReactEventsView
• ReactMeasuresView (includes modifications to address Hide empty lanes #71)
• FlamegraphView
• LaneLabelView (envisioned, for Show React version #73)
• UserTimingMarksView (envisioned, for Show remaining User Timing marks in the UI  #72)
• Other content views that provide more context around the profile, e.g. user interaction events, network events, etc.

These are instantiated in CanvasPage. Their code is a light-ish adaptation of the existing code in renderCanvas, but in the future we may want to turn ReactMeasuresView and FlamegraphView into a vertically stacked StaticLayoutView to reuse more View logic.

Key flows

Rendering

1. When the surface's displayIfNeeded method is invoked, it calls its root view's displayIfNeeded method.
2. If a view's needsDisplay or subviewsNeedDisplay flags are true and the view is visible onscreen, it will layout its subviews and draw.
3. Views are responsible for calling their subviews' displayIfNeeded methods.

Event handling

1. Browser event listeners are registered in useCanvasInteraction. useCanvasInteraction is given a callback that invokes a Surface's handleInteraction method.
2. useCanvasInteraction translates browser events into higher-level interactions (e.g. mouse wheel event -> zoom or scroll interactions). Note: The current implementation is a WIP and is neither complete nor correct.
3. useCanvasInteraction calls its interactor callback. Our interactor is set in CanvasPage, which calls the surface's handleInteraction method.
4. The surface forwards the interaction to its root view's handleInteractionAndPropagateToSubviews method. A view may handle the interaction, but it must propagate the interaction to its subviews.

State updates

1. When a view needs to be redrawn/laid out, its setNeedsDisplay method should be called. This is often done in superviews' layoutSubviews or in event handlers.
2. setNeedsDisplay sets the view's needsDisplay to true, and recursively invokes all its superclasses' setSubviewsNeedDisplay. This ensures that the next render will be able to efficiently find all views that need to be redrawn.

Test Plan

1.4k lines of new code and no tests 🙃

• yarn lint
• yarn flow (has errors in untouched code, but none in the new code)
• yarn test (tests untouched, but still passing)

Manually tested on:

• Chrome on Windows with a mouse
• Chrome, Firefox, Safari on macOS with a trackpad

60 FPS on the Facebook.com profile 😍

TODO

To be addressed in this PR

• Viewport resizes don't resize the canvas
• If the viewport is larger than the root view, there's a big black unpainted area.
• Hovers are not implemented
• Zooms are not implemented
• Scrolling is not implemented
• HorizontalPanAndZoom:
  • Allows content to be panned offscreen.
  • Does not cancel panning if there was no mouse up event.
• Delete old code
• Canvas resizes that don't change a view's frame/visibleArea don't get painted and leave a black area (canvas resizes should force needsDisplay on all views)
• Bug: it's possible to zoom out too far, leaving a black area on the right of the data (Force content-width based minimum zoom in HorizontalPanAndZoomView #83)

To be fixed in future PRs (will be convert most to issues)

• Zoom to batch is not implemented (Reimplement Zoom to batch context menu item #84)
• Consider merging horizontal and vertical pan/scroll/zoom views
• Consider moving subview storage and management to View (Views: Support subviews in View class #95)
• ResizableSplitView (Add vertical resizer between canvas #19)
• Documentation (docstrings + higher level contributing docs) (Add documentation for views abstraction #85)
• Tests (Add tests for views #86)

[vercel]

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/gs678bh9d
✅ Preview: https://scheduling-profiler-prototype-git-eliang-uiview.mlh-fellowship.vercel.app

[taneliang]

@jevakallio this is ready for an approach/architecture review :)

I've described the high level goals, broken down the classes and described the key behaviors. At this point, This PR is roughly half complete, where completion = feature/functional parity with the current profiler + #71. I'd love to get your thoughts on this, especially regarding these questions:

1. Is this view approach the right one/a good one? I think this is a good approach and it'll work well, but because it is a bit alien for web developers it may be a bit strange.
2. Do you have any other approaches/architectures that you think will be a better fit for this tool?

[taneliang]

Thanks @jevakallio for the call! Key points brought up during the call:

1. View composition should be transparent to the composed view; composed views should not need to care about what their superviews are. This is a design goal.
2. This abstraction layer and approach make sense from a technological standpoint.
3. Con: this approach reimplements a few things that would normally just be handled by the browser, especially our event handling system.
4. Documentation needs to be good because this abstraction layer is not very "webby".

[taneliang]

Hey @bvaughn, I'd like to get your thoughts on the approach of this PR.

Essentially, I'm thinking of replacing the current renderCanvas code with a thin-ish abstraction layer inspired by iOS's UIViews.

• Currently: renderCanvas is largely unchanged from the one in your original prototype; it is a function that draws the canvas, and mouse events are handled separately using usePanAndZoom and getHoveredEvent.
• After this PR: With the new view system, the different parts of the canvas (e.g. flamegraph, React measures, React events, etc) and layouty things (e.g. scroll views, stacked layouts) will be encapsulated in individual view classes along with related interaction handling. There are a list of these classes in the PR description.

My goals are:

1. To be performant.
2. To improve our performance by only draw regions that need drawing.
3. Make iteration easier by encapsulating draw + layout + interaction code in view classes.

My concerns:

1. Because such a view-based system is not common in the web ecosystem, it may make it more difficult for future contributors to understand.
2. @jevakallio also mentioned that this approach reimplements a few things that would normally just be handled by the browser, especially our event handling system.

Although I think the benefits of this view approach outweigh these cons, I'd like to get your feedback on this before I proceed further in this direction :)

We can talk about this more during our call on Monday and I can walk you through this then, but for now the PR description will hopefully describe this in greater detail. Specifically, I'd like to know if you think this view-based system is a good approach, and whether you have any requirements/suggestions.

[bvaughn]

Hi @taneliang,

I'm likely not going to have a chance to review this code today because my schedule is pretty full. Happy to talk through it with you on Monday morning though.

At a high level, the approach you're describing makes sense- memoizing/limiting the re-rendering work- although I have some concern when reading through the classes section of the PR. Using inheritance for different behaviors feels like it might be problematic. The classes designed seemed overly narrow. For instance, I imagine that ReactMeasuresView section would want behaviors from both HorizontalPanAndZoomView and ResizableSplitView. Seems like composition might be a better way of adding these behaviors.

I'm also not sure what an example of VerticalScrollView would be. Could you elaborate a bit on your plans for this? All views would need to scroll (vertically and horizontally) together, right? Are you imagining a change in the scrolling UX?

We can talk more on Monday 😄 Thanks for the ping and the detailed description!

[taneliang]

Thanks @bvaughn!

Seems like composition might be a better way of adding these behaviors.

Ah, I think I should've clarified this better. The behaviors are currently composed by nesting views, and so our content views (e.g. ReactMeasuresView) all inherit from the base View class. We then have layouty views (HorizontalPanAndZoomView, VerticalScrollView, StaticLayoutView, ResizableSplitView) that manipulate their subviews' frames, so you'll be able to nest a bunch of these views and our content views to achieve more complex layouts and interactions. These layouty views are not meant to be subclassed.

The Vercel deploy preview currently has our content views in a vertical stack StaticLayoutView, which is nested in a HorizontalPanAndZoomView. This implements the pan+zoom behavior of the current UI without the vertical scroll and trackpad scroll (but with some additional bugs). Once VerticalScrollView is implemented, we should be able to just nest everything we have inside it to get vertical scrolling.

This is definitely not as clean a design as I'd hoped; I'd have preferred to inject behaviors into a View class via drawer/layout/interactor functions, which should be more flexible that what I have now. Unfortunately, when I tried that it was just too complicated so I stuck with the current approach 😆

I'm reasonably confident that this design works and may be good enough™️ since behaviors are also composed in a similar way in UIKit. HorizontalPanAndZoomView and VerticalScrollView in particular are also modeled after UIKit's UIScrollView.

Are you imagining a change in the scrolling UX?

Nope, this architecture will be able to do everything that we can do now.

On top of that, this design is also partially intended to also make it easier to support more complex UIs and interactions, including having the lanes area scroll separately from the flamechart area. There are some implementation details to be sorted out to ensure that the horizontal pan and zooms remain synced between the two views (even though they're in the same canvas, the views maintain their own pan and zoom state), but that should be relatively easy to solve.

Let's talk more about this on Monday; it'll probably be easier to talk through these ideas then. Thanks for looking at this despite your packed schedule! Have a great weekend 😄

[jevakallio]

Rubberstamp approved, as per discussion on Discord

[bvaughn]

Woo hoo! 🥳