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

Accessing strokeStyle and fillStyle of drawing context slow #2011

Closed
3 of 15 tasks
wxs opened this issue Jun 20, 2017 · 3 comments
Closed
3 of 15 tasks

Accessing strokeStyle and fillStyle of drawing context slow #2011

wxs opened this issue Jun 20, 2017 · 3 comments

Comments

@wxs
Copy link
Contributor

wxs commented Jun 20, 2017

Nature of issue?

  • Found a bug
  • Existing feature enhancement
  • New feature request

Most appropriate sub-area of p5.js?

  • Color
  • Core
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL

Which platform were you using when you encountered this?

  • Mobile/Tablet (touch devices)
  • Desktop/Laptop
  • Others (specify if possible)

Details about the bug:

  • p5.js version: 0.5.11
  • Web browser and it's version: Chrome Version 61.0.3128.0 (Official Build) dev (64-bit)
  • Operating System: OS X 10.11.6
  • Steps to reproduce this: Repeatedly call "vertex"

In profiling a sketch I'm working on that makes heavy use of vertex, I found that calls were quite slow, and specifically due to the calls internally to _getStroke and _getFill (see screenshot).

screen shot 2017-06-20 at 13 33 54

It seems that its slow for it to access the fillStyle and strokeStyle of the drawing context. Although I don't know why this would be the case, I found that by locally caching these values, and then updating the cache in every place where they are set, performance improves significantly:

p5.Renderer2D.prototype._getFill = function(){
  return this.cachedFillStyle;
};

p5.Renderer2D.prototype._setFill = function(fillStyle){
    this.drawingContext.fillStyle = fillStyle
    this.cachedFillStyle = this.drawingContext.fillStyle;
};

p5.Renderer2D.prototype._getStroke = function(){
  return this.cachedStrokeStyle
};

p5.Renderer2D.prototype._setStroke = function(strokeStyle){
    this.drawingContext.strokeStyle = strokeStyle
    this.cachedStrokeStyle = this.drawingContext.strokeStyle;
}

this will, of course, fail if someone tries to modify the stroke or fill style outside of these helper functions. There may be another solution by modifying the part of the vertex that calls this.

    vert[5] = this._renderer._getFill();
    vert[6] = this._renderer._getStroke();

If the helper function-based approach looks good, I can probably package up my changes as a PR. Let me know!

@limzykenneth
Copy link
Member

Do you think you can create a simple sketch that we can stress test this problem on? Something that maybe calls vertex a lot of times and log how long it takes, maybe even one that uses the fix you proposed so we can compare. Thanks.

@wxs
Copy link
Contributor Author

wxs commented Jun 23, 2017

Ok yes I'll put that together now.

I've also realized that things are still worse when in TRIANGLES mode because the renderer keeps re-setting the fill and the stroke for each triangle. This has the advantage that you can in one beginShape() have multiple different fill colours, but that's not documented as a possible behaviour anyway and it significantly reduces performance.

This is trivially fixed by just defining a shape to have a single colour. The more general approach would be similar: caching the stroke and fill and not actually modifying the drawingContext if they've not changed, but it becomes important that nobody externally accesses the drawingContext except through these methods.

@wxs
Copy link
Contributor Author

wxs commented Jun 23, 2017

Ok so I've got a tentative fix up as a PR here: #2021

Here's an example sketch where I get huge framerate improvement with this change: https://gist.github.com/wxs/d95a1239fad5cafd8eacf8417d2a137f

and here are two profiler screen caps showing the difference

slow

fast

as you can see, the original slow version is spending a huge amount of time just getting the current stroke or fill in the vertex calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants