[Webkit-unassigned] [Bug 200456] [results.webkit.org Timeline] Performance improvement - Skip render offscreen canvas

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=200456

--- Comment #5 from Zhifei Fang <zhifei_fang at apple.com> ---
(In reply to Jonathan Bedard from comment #3)
> Comment on attachment 375860 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=375860&action=review
> 
> > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:121
> > +class ColorBatchRender {
> 
> While you are generally rendering things by color, I don't see this class
> actually setting colors. It's more accurately a generic render batch map
> that you happen to be using colors as a key. I also don't get why you need
> to pass colors in.
> 
> With that in mind, I think that you need to pick. Either this class batches
> together renders by color (and so should set colors, but does not need to
> pass them to the callbacks) or, this is a generic batch mapping which maps
> any object to a set of associated render callbacks. In that case, it would
> seem that the very first function provided would define what sort of context
> you were defining (ie, is this about filling with a specific color, drawing
> lines with a specific color, using a specific font, etc)
> 
> >

The operation can be fill color or stroke color or do those together, so the user of this class should define the start operation and end operation, in which they can set strokeStyle, fillStyle or both of them. I don't want to do this for user, because setting all of them together, and what's more, there are more styles should be set like lineWidth/ and beginPath, we should provide a hook for user to define their operation at the beginning and the end of the batch. 
 Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:208
> >                  context.fillStyle = color;
> 
> I'm surprised to see you setting the fillStyle in this function, I would
> have expected it to be set in the ColorBatchRender class

This is because we have fillStyle and also strokeStyle
> 
> > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:375
> > +                // Clean the canvas, free its memory
> 
> Do we really need to do this? It feels like a WebKit bug if we do.
> 
> > Tools/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:639
> > +            context.strokeStyle = usedLineColor;
> 
> Shouldn't this be color instead of usedLineColor?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190812/a380746c/attachment.html>