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

Boost fails with 6.0.4/6.0.3 when webGL is disabled, works with 6.0.1 #7610

Closed
manu-st opened this issue Jan 4, 2018 · 13 comments
Closed

Boost fails with 6.0.4/6.0.3 when webGL is disabled, works with 6.0.1 #7610

manu-st opened this issue Jan 4, 2018 · 13 comments
Assignees

Comments

@manu-st
Copy link

manu-st commented Jan 4, 2018

Expected behaviour

Graph should be rendered.

Actual behaviour

Graph is not rendered and the following exception is thrown:

Cannot read property 'width' of undefined in renderCanvas line 250

Live demo with steps to reproduce

We use highstock and use it as following:

const highcharts = require ("highcharts/highstock.src");
require ("highcharts/modules/boost-canvas.src") (highcharts);
require ("highcharts/modules/boost.src") (highcharts);

Affected browser(s)

Using electron 1.7.9 with webGL disabled by default.

@pawelfus
Copy link
Contributor

pawelfus commented Jan 4, 2018

Hi @manu-st

Thanks for letting us know about the issue. Is this issue connected to Electron v1.7.9 only? Does it work for you in browsers? For example: http://jsfiddle.net/1f2q8e0e/2/ (with disabled webgl of course).

Internal note:
I think this is the line responsible for this error:

ctx.clearRect(0, 0, this.canvas.width, this.canvas.height);

b4b4221#diff-9428541d6e9a9f2d0ef8110acbcb1629R346 - added in merge commit.

@manu-st
Copy link
Author

manu-st commented Jan 4, 2018

I'm not sure if this issue is related to electron v1.7.9 but I mentioned it as this is the version we are using.

This is the line that is causing me the issue. If I load the fiddle under electron, the graph doesn't show up but no exception occurs in the console.

@manu-st
Copy link
Author

manu-st commented Jan 4, 2018

Just realized that I took the wrong fiddle. When I use the correct one, it seems to work in my instance of electron when webgl is disabled.

@pawelfus
Copy link
Contributor

pawelfus commented Jan 4, 2018

Thanks for the details. That suggests two possible issues:

  • you have outdated version of Highcharts, but you mentioned you have tested 6.0.3/6.0.4 so this is not a problem
  • it's connected to the chart config. Could you share your options? You can edit my jsFiddle (and check at the same time) and check if you are able to recreate the issue

@manu-st
Copy link
Author

manu-st commented Jan 4, 2018

It will take some time but I'll try to reproduce the issue. Thanks!

@TorsteinHonsi
Copy link
Collaborator

TorsteinHonsi commented Jan 5, 2018

@pawelfus Your fiddle actually runs on WebGL. In order to force it to run canvas, we need to delete the webGL rendering context so that Boost's feature sniffing is tricked.

delete window.WebGLRenderingContext

http://jsfiddle.net/highcharts/1f2q8e0e/3/

@pawelfus
Copy link
Contributor

pawelfus commented Jan 5, 2018

@TorsteinHonsi - I simply disabled WebGL in my Chrome to test the demo, but thanks for the general one!

@sebastianbochan
Copy link
Contributor

Additional demo from #7670:

@achin
Copy link

achin commented Jan 16, 2018

Thanks, @sebastianbochan. It looks like this may be caused because the Charts.renderSeries() call doesn't pass a context to each().

https://github.com/highcharts/highcharts/blob/master/js/parts/Chart.js#L1662

Changing the code to the following allows both one and two series to be rendered, though I'm not sure if this affects anything else.

	renderSeries: function () {
		each(this.series, function (serie) {
			serie.translate();
			serie.render();
		}, this); // <- added 'this'
	}

I apologize, but I'm not familiar with your codebase, tests, or development process. Would you have time to fix this in an upcoming release, or would you prefer a pull request?

@sebastianbochan
Copy link
Contributor

@cvasseng what do you think?

@achin
Copy link

achin commented Jan 16, 2018

Apologies. I jumped to conclusions about the fix. It turns out WebGL was still enabled, even though I tried disabling it.

I'll continue trying to figure out what's going on.

@cvasseng
Copy link
Contributor

cvasseng commented Jan 16, 2018

The issue was with clearing and copying the render result. Basically, it always tried to use a local canvas on the series, which isn't always there (actually it's never there when all series can be boosted and one or more are unless boost.allowForce is set to false).

It's been fixed in bugfix/7610-boost-canvas-rendering.

Demo of the fix: https://jsfiddle.net/e63x95xg/2/

Pull request added.

@achin
Copy link

achin commented Jan 16, 2018

Thanks, cvasseng.

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

No branches or pull requests

6 participants