Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Moving canvas text into a plugin #935

Merged
merged 24 commits into from

4 participants

@dnschnur
Owner

This is my project to revert Flot to HTML text, while enabling canvas text via a plugin. It still requires cleanup, optimization, and testing in several areas, but I'd appreciate any feedback on the design, tradeoffs, whether the change is even worth it, etc. My goals were:

  1. Revert to HTML/CSS text
  2. Add a plugin to draw text (and eventually other things, like the legend) to the canvas
  3. Generalize text drawing so it can be used for axis labels, titles, etc.
Design

I started by factoring some of Flot's canvas management code out into a new class called Canvas. While I don't want to go overboard with objects, I would like to see a little more encapsulation in the codebase. This allowed me to remove some duplicate code.

I then gave the Canvas class two methods: drawText and getTextInfo:

  • getTextInfo(text, font, angle)
  • drawText(x, y, text, font, angle, halign, valign)

The base implementation of getTextInfo creates a new div, assigns it font/style via either class or inline style, measures its dimensions, then saves the dimensions and div HTML in a cache keyed on the text, font and angle.

The base implementation of drawText uses getTextInfo to load an info object, calculates horizontal/vertical alignment based on the text's dimensions, then appends the div HTML to a string buffer.

The Canvas class has a method render() that is called on redraw. It creates, if it doesn't already exist, a new div in front of the canvas to act as a text layer. It then dumps the buffer full of text divs into this layer.

Plugins now receive an object holding Flot's classes - currently just Canvas - as part of their init method. This gives them deeper access to the library's internals than the existing hooks system.

The canvas text plugin uses this to override drawText and getTextInfo with its own implementations. Its getTextInfo splits the text into lines, uses canvas measureText to find the dimensions of each line, and saves the results as an info object in the same kind of cache as the base implementation. Its drawText method uses getTextInfo to load the info, then draws each line separately using canvas fillText.

This is mostly the same code as already exists in 0.8.0-alpha, with one key difference: if the font is provided as CSS styles, and a font-spec object is not provided, getTextInfo will create a dummy element and derive the font definition from its styles. This is quite nice, since it means that a regular HTML-text plot can be redrawn using canvas text without any other changes in options.

Advantages

The abstraction allows the same code to be re-used for all text drawing, not just axis labels. This should make it trivial to implement axis labels, plot titles, and data labels in the future.

The way in which the canvas plugin overrides these methods lets the change transparently affect all text, and can be toggled via an option. The canvas plugin's ability to use CSS makes this transition even cleaner.

HTML text makes it possible to embed mark-up in labels (#772), though it's not clear how often this is useful. It allows text to be styled via CSS, which is desirable for some users, though probably not critical. It enables the use of em units in font sizes (#898), plus various other features like letter-spacing and text-shadow.

HTML text is reliably good-looking; some browsers render canvas text with inferior quality (#789), and others (#71, #815) have bugs that cause measureText to return inaccurate values.

The fact that HTML text is vector-based makes it look nicer when exporting the page to PDF (#913), although there may be work-arounds to that particular problem.

Disadvantages

The abstraction greatly reduces performance in the base case, since it prevents us from using the 0.7 optimization where all labels from each axis are added to a single div and measured together. It is equally slow with canvas drawing enabled if CSS classes are used, since it must still add every element to the DOM individually to query styles.

On the other hand there are normally not many axis labels, and the use of a cache mostly eliminates the performance hit for plots that are frequently redrawn.

It is also slower than 0.7 in that labels are cleared and re-created on every redraw, instead of only when setupGrid is called. This also prevents external interaction with the axes, since the divs are destroyed on redraw. The move to canvas text in 0.8.0-alpha already had both of these effects, though, and over many months nobody has complained, so it's probably not a big deal.

A canvas-only implementation is naturally cleaner in code than one that needs to switch.

dnschnur added some commits
@dnschnur dnschnur Renamed the 'canvas' variable to 'surface'.
Renaming the variable gives us room to create a new class called Canvas.
f66c9ae
@dnschnur dnschnur Abstract-out canvas creation into an object.
Moved canvas creation and size management into a new Canvas class.

This is the first step towards a more object-oriented architecture.
Since we create multiple canvases, and have to maintain several
module-global variables to track their properties, they are the ideal
place to start.

This commit also removes sizing code that was duplicated between
makeCanvas and resizeCanvas.
a9be4d5
@dnschnur dnschnur Added a basic frame for the canvas-drawing plugin. 42d5592
@dnschnur dnschnur Added methods to draw and measure text.
These methods provide a common way to draw HTML text above a canvas.

The getTextInfo method generates div HTML for text with a given font
style/class and angle, measures the element's dimensions, and saves
everything in a cache.  The drawText method takes the resulting entry,
finishes generating the inline styles necessary to position the div, and
adds the result to a buffer.  The render method dumps the buffer into an
overlay and expires unused cache entries.
edc2bbd
@dnschnur dnschnur Provide a way for plugins to override classes. 3b2d43b
@dnschnur dnschnur Moved canvas tick rendering into a plugin.
The base implementation uses the new drawText and getTextInfo methods to
draw text in HTML.  Canvas rendering has been moved to overrides of
these methods within the canvas-render plugin.
a0529ee
@dnschnur dnschnur was assigned
@OleLaursen
Owner

Dump of my thinking:

  • Canvas class: initially I'm thinking urgh, mostly because it's an extra layer, but when I look into it, it seems fine, given the annoying browser bugs.

  • Speed: what matters is probably absolute numbers. With Firefox 10 on my i5, it takes around 7 ms per axis on the basic example by my count (8-16 ms for IE 7 in a VM). That's probably fine. Alternatively, another idea could be to keep the measured labels around in the DOM (hidden) in anticipation of them actually being displayed soon, that probably doesn't speed up measurements, but it could speed up the following draw operation.

  • Cache: I think your cache handling is clever. The only problem I can see if something changes underneath (e.g. changes in CSS), so it turn out to be necessary with some way of clearing it.

  • setupGrid() - perhaps this has simply outlived its former usefulness :)

Overall, I think this is great work. It's too bad the HTML5 spec dudes and implementors screwed up on

dnschnur added some commits
@dnschnur dnschnur Replace axis.font with options.font.
Instead of giving the axis its own font property, we simply look at its
options, where the font comes from in the first place.  A separate
property is unnecessary and inconsistent with the way other axis options
are handled.
c36b344
@dnschnur dnschnur Fixed missing/superfluous semicolons. d2642e8
@dnschnur dnschnur Preserve canvas elements on re-plot.
Since the Canvas .text object is jQuery-wrapped, it was not preserved as
expected when clearing the canvas of junk.  I've replaced the selection
with one based on element classes.
98b6361
@dnschnur
Owner

@OleLaursen In an earlier iteration I did try holding the actual elements in the cache, which is significantly faster. That caused difficulties with regards to the cache, though, since more than one element can share the same text/font/angle (if the same value appears on the x and y axes, for example) yet require distinct elements.

You're correct that the cache needs to be refreshed when the underlying CSS changes. I need to think about that, since it is kind of a liability; best case we'd have to expose some method to the user, which is really not ideal.

So perhaps I need to think harder about the cache, and whether there's a better way to do this. Thanks for the review!

@lothar7

First I commend you guys on a lot of hard work towards v0.8. I have not been testing out the development version too much so I apologize for coming into this discussion late.

I an very happy that you guys have kept the axis labels as html. However I am not happy if its slower than the HTML axis labels i 0.7. If you are using older browsers like IE8 then this is quite a significant problem when drawing realtime trends (at like 10Hz). I was actually quite happy with the labels in v0.7 and thought that implementation worked great.

Another thing I have noticed is that there is can be flickering issues on the iphone when recreating html labels at every update. This solution wont make this problem smaller.

The ability to interact with the axis is essential in my opinion and I use it quite extensively in my trendtool developed for a Plant Information Management System.

I use my own "patched" up version of 0.7 with the hope to migrate to 0.8 when its ready. However if I understand this implementation properly then this would be a step back because the disadvantages far outweigh the advantages IMHO. I am not sure I can use this implementation of v0.8 if this turns out to be the final version.

@dnschnur
Owner

Thanks; this is exactly the kind of feedback I was looking for.

@lothar7

In an ideal world it would be good to create the axis labels just once and then update the plot and grid by just moving them when the different axis change position (as long as the scale is the same) and just create new ones as required and delete old ones when they are no longer inside the view. It would be like a sliding window and any labels still inside the window would just change its coordinate as long as the scale stays the same. This would take care of the realtime problem, since most labels stay visible on the plot for a while. Especially the y axis labels, which in most cases are quite stationary, unlike the x axis.

If done this way, then having a separate div for each label would not be such a big problem, although some kind of wrapper div to support click on the axis would still be required.

This way of doing it could work since there is already a setupGrid method, which could remember the previous state and then make the appropriate decision as to recreate all the labels or just move the existing ones. Of course if the css or any other text or information change relating to the label everything must be created. I have no problem with calling setupGrid after modifying css or anything related. Changing the axis scale already requires calling setupGrid so its perfectly sensible to expect the user to call setupGrid if css related stuff changes.

I guess my point is to optimize the most performance critical part of the code which would be the realtime rendering scenario. If creating a plot from scratch is a little bit slower that is less of a big deal.

dnschnur added some commits
@dnschnur dnschnur Cache actual elements instead of buffering HTML.
This significantly improves performance, since we already create the
elements when measuring them, and that effort is now no longer wasted.
We must take care to detach, rather than remove, when clearing the text
layer, so we can add the elements back later if necessary.
73baa2b
@dnschnur dnschnur Move cached hasOwnProperty to the top level. ff0e5c1
@Globegitter

I have a question, that might just be partially related to this. So if I want to add labels to barcharts (in the canvas), which show the exact y-value of each bar, how would that be possible?
The way I am currently doing is, just overlaying div's, which works theoretically fine, but I want to convert everything to a PNG on the client side, and this is much more painful if not everything is done via canvas. Really appreciate any help.

@dnschnur
Owner

@Globegitter Currently the only way to get that functionality is to write your own plugin, with canvas drawing calls adding the labels. These changes, once finalized, will make that plugin much smaller and easier to write, to the point where we might consider even just folding it into core.

@Globegitter

@dnschnur Ok thanks for the info. Unfortunately don't have time to spend any extensive time on that so will just leave it how I currently have it.

dnschnur added some commits
@dnschnur dnschnur Replace drawText with add and remove methods.
Every cache element now contains the actual text element instead of just
its HTML, plus a flag indicating whether it is visible.  The addText and
removeText methods control the state of this flag, and the render method
uses it to manage elements within the text container.  So where we
previously used drawText to actually render text, now we add each string
once, then let the render method take care of drawing them as necessary.

This dramatically improves performance by eliminating the need to clear
and re-populate HTML text on every drawing cycle.  Since the elements
are now static between add/remove calls, this also allows users to add
interactivity, as they could in 0.7.  Finally, it eliminates the need
for a separate 'hot' cache.

I also removed the unnecessary 'dimensions' object; it's easier and
faster to store the width and height at the top level of the info
object.
a9a3164
@dnschnur dnschnur Reverse cache key order to ensure uniqueness.
Also switch from dashes to pipes, and remove the angle for now, since we
don't currently support rotated text.
a036aa9
@dnschnur dnschnur Factor out text layer creation to its own method.
This sets the stage for allowing the use of multiple layers.
e7de873
@dnschnur dnschnur Add text to its actual layer before measuring it.
The getTextInfo method previously added new text to the top-level
container when measuring it.  Now it adds the text to the text layer,
just as it will be when rendered, so that parent-child CSS rules can
resolve correctly.

This also avoids having to safe a reference to the top-level container,
since it wasn't used anywhere else.
4203a66
@dnschnur dnschnur Allow text to be divided between multiple layers.
This lets users 'namespace' text more naturally, i.e. placing x-axis
labels in a different container from y-axis labels, providing more
flexibility when it comes to styling and interactivity.

Internally the text cache now has a second tier: layers > text > info.
77e50b1
@dnschnur dnschnur Break text styles into their own cache tier.
Previously the cache was divided only by layer, with entries keyed on a
string built from the text and style.  Now the style has its own tier in
the cache, i.e. layers > styles > text > info.

This introduces some complexity, since the nested for loops are ugly,
but at the same time we avoid having to create the cache-key strings.
More importantly it solves the problem of uniqueness that exists when we
try to join strings that may contain arbitrary text.  It also allows a
further optimization in the canvas plugin, which can now set text style
and color just once per distinct style, instead of with every string.
a2dd064
@dnschnur
Owner

Here's the second revision of my canvas-text project. This replaces the drawText method with addText and corresponding removeText methods, and enables the creation of multiple text layers.

I realized that the main problem with my earlier revision was that axis labels in 0.8 were too tightly bound to the idea of canvas text; everything was set up to re-render on every call to draw.

So what I did is to have text cache entries hold actual elements, as Ole suggested, along with a flag indicating whether they should be visible. The add/remove methods only control the state of this flag; it's up to the render method to decide what to do with it. The core implementation appends/detaches the elements from their layer, while the canvas implementation renders on every pass just as before.

This dramatically improves redraw performance, since we no longer re-create elements on every pass, and even improves first-draw performance, since the elements are re-used from when we create them to measure dimensions. It's still not as fast as 0.7, but comes closer, and is equal or faster on redraw.

Most importantly, HTML elements are once again persistent, as they were in 0.7; they change only on a call to setupGrid. In fact this works even better than in 0.7; the cache ensures that identical elements aren't touched at all. This restores the ability to set up interactions on text elements, and being able to place elements in different containers, i.e. flot-x-axis and flot-y-axis, makes that even easier.

For now this ignores the question of what to do to cached values when styles change. Since I don't see that happening often in practice, I'm going to skip it for now, but this design makes it easy to add an update method.

I'm much happier about this revision; it addresses my concerns and all the feedback that I've received, and just feels better overall. We get to keep the benefits of my abstraction without sacrificing anything on the HTML side, and the performance hit is now much lighter.

@brianpeiris brianpeiris referenced this pull request
Closed

JSHint compliance #974

dnschnur added some commits
@dnschnur dnschnur Minor cleanup of text-style color defaults. e354071
@dnschnur dnschnur Give tick labels the 'tickLabel' class.
The tickLabel class is deprecated in favor of flot-tick-label, but we'll
continue to use it until the release of version 1.0.0, for
backwards-compatibility.
bb0acac
@dnschnur dnschnur Updated the API docs for axis text changes. 27c7011
@dnschnur dnschnur Add back legacy styles for tick label containers.
These styles are deprecated, but we'll continue to use them until the
release of version 1.0.0, for backwards-compatibility.
0df6bc4
@dnschnur dnschnur Updated NEWS for axis text changes. 7a799ba
@dnschnur dnschnur Updated credits for canvas text support. 39698d3
@dnschnur dnschnur merged commit 60ed6b2 into flot:master
@dnschnur dnschnur deleted the dnschnur:canvas-text branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 19, 2013
  1. @dnschnur

    Renamed the 'canvas' variable to 'surface'.

    dnschnur authored
    Renaming the variable gives us room to create a new class called Canvas.
  2. @dnschnur

    Abstract-out canvas creation into an object.

    dnschnur authored
    Moved canvas creation and size management into a new Canvas class.
    
    This is the first step towards a more object-oriented architecture.
    Since we create multiple canvases, and have to maintain several
    module-global variables to track their properties, they are the ideal
    place to start.
    
    This commit also removes sizing code that was duplicated between
    makeCanvas and resizeCanvas.
  3. @dnschnur
  4. @dnschnur

    Added methods to draw and measure text.

    dnschnur authored
    These methods provide a common way to draw HTML text above a canvas.
    
    The getTextInfo method generates div HTML for text with a given font
    style/class and angle, measures the element's dimensions, and saves
    everything in a cache.  The drawText method takes the resulting entry,
    finishes generating the inline styles necessary to position the div, and
    adds the result to a buffer.  The render method dumps the buffer into an
    overlay and expires unused cache entries.
  5. @dnschnur
  6. @dnschnur

    Moved canvas tick rendering into a plugin.

    dnschnur authored
    The base implementation uses the new drawText and getTextInfo methods to
    draw text in HTML.  Canvas rendering has been moved to overrides of
    these methods within the canvas-render plugin.
Commits on Jan 31, 2013
  1. @dnschnur

    Replace axis.font with options.font.

    dnschnur authored
    Instead of giving the axis its own font property, we simply look at its
    options, where the font comes from in the first place.  A separate
    property is unnecessary and inconsistent with the way other axis options
    are handled.
  2. @dnschnur
  3. @dnschnur

    Preserve canvas elements on re-plot.

    dnschnur authored
    Since the Canvas .text object is jQuery-wrapped, it was not preserved as
    expected when clearing the canvas of junk.  I've replaced the selection
    with one based on element classes.
Commits on Feb 16, 2013
  1. @dnschnur

    Cache actual elements instead of buffering HTML.

    dnschnur authored
    This significantly improves performance, since we already create the
    elements when measuring them, and that effort is now no longer wasted.
    We must take care to detach, rather than remove, when clearing the text
    layer, so we can add the elements back later if necessary.
Commits on Feb 17, 2013
  1. @dnschnur
Commits on Feb 21, 2013
  1. @dnschnur
Commits on Feb 23, 2013
  1. @dnschnur

    Replace drawText with add and remove methods.

    dnschnur authored
    Every cache element now contains the actual text element instead of just
    its HTML, plus a flag indicating whether it is visible.  The addText and
    removeText methods control the state of this flag, and the render method
    uses it to manage elements within the text container.  So where we
    previously used drawText to actually render text, now we add each string
    once, then let the render method take care of drawing them as necessary.
    
    This dramatically improves performance by eliminating the need to clear
    and re-populate HTML text on every drawing cycle.  Since the elements
    are now static between add/remove calls, this also allows users to add
    interactivity, as they could in 0.7.  Finally, it eliminates the need
    for a separate 'hot' cache.
    
    I also removed the unnecessary 'dimensions' object; it's easier and
    faster to store the width and height at the top level of the info
    object.
Commits on Feb 24, 2013
  1. @dnschnur

    Reverse cache key order to ensure uniqueness.

    dnschnur authored
    Also switch from dashes to pipes, and remove the angle for now, since we
    don't currently support rotated text.
  2. @dnschnur

    Factor out text layer creation to its own method.

    dnschnur authored
    This sets the stage for allowing the use of multiple layers.
  3. @dnschnur

    Add text to its actual layer before measuring it.

    dnschnur authored
    The getTextInfo method previously added new text to the top-level
    container when measuring it.  Now it adds the text to the text layer,
    just as it will be when rendered, so that parent-child CSS rules can
    resolve correctly.
    
    This also avoids having to safe a reference to the top-level container,
    since it wasn't used anywhere else.
  4. @dnschnur

    Allow text to be divided between multiple layers.

    dnschnur authored
    This lets users 'namespace' text more naturally, i.e. placing x-axis
    labels in a different container from y-axis labels, providing more
    flexibility when it comes to styling and interactivity.
    
    Internally the text cache now has a second tier: layers > text > info.
  5. @dnschnur

    Break text styles into their own cache tier.

    dnschnur authored
    Previously the cache was divided only by layer, with entries keyed on a
    string built from the text and style.  Now the style has its own tier in
    the cache, i.e. layers > styles > text > info.
    
    This introduces some complexity, since the nested for loops are ugly,
    but at the same time we avoid having to create the cache-key strings.
    More importantly it solves the problem of uniqueness that exists when we
    try to join strings that may contain arbitrary text.  It also allows a
    further optimization in the canvas plugin, which can now set text style
    and color just once per distinct style, instead of with every string.
Commits on Mar 3, 2013
  1. @dnschnur
  2. @dnschnur

    Give tick labels the 'tickLabel' class.

    dnschnur authored
    The tickLabel class is deprecated in favor of flot-tick-label, but we'll
    continue to use it until the release of version 1.0.0, for
    backwards-compatibility.
  3. @dnschnur
  4. @dnschnur

    Add back legacy styles for tick label containers.

    dnschnur authored
    These styles are deprecated, but we'll continue to use them until the
    release of version 1.0.0, for backwards-compatibility.
  5. @dnschnur
  6. @dnschnur
Something went wrong with that request. Please try again.