Refactor gl3d scene

[archmoj]

This PR adds setters to gl-plot3d and refactors functions/methods of gl3d/scene.js file to use gl-plot3d functions to change camera attributes and avoid two camera instances, etc.

@plotly/plotly_js
Please review this PR commit by commit.

After all, this PR does NOT f_i_x #4236 by passing a different config (premultipliedAlpha: false) to gl-plot3d module. As used to be suggested in this demo {to compare before and after effects on Windows(7/10) using Chrome/Firefox/Opera as well as on Ubuntu using Firefox}.
That may be addressed in separated PR in future.

[etpinard]

Hmm. Does this lead to any changes in performance ?

[archmoj]

Good question. It shouldn't. Anyway I would double check.

[archmoj]

We decided not to change this config in this PR for now: ade22a6.

[etpinard]

Fix #4236 by passing different config to gl-plot3d module.

Is the only change in the WebGL config the premultipliedAlpha value?

[archmoj]

Fix #4236 by passing different config to gl-plot3d module.

Is the only change in the WebGL config the premultipliedAlpha value?

Yes that is the only change in the config.

[archmoj]

before vs after windows10+chrome78

[archmoj]

before vs after windows10+chrome78

[archmoj]

before vs after windows10+firefox70

[archmoj]

Also added a codepen to the PR description to help compare the two options.
After figuring out which premultipliedAlpha option to use as the default, it may be best to provide both options in the API.
Anyway applying premultipliedAlpha: false would make the graphs render similar to save as png version.
cc: @alexcjohnson @antoinerg

[archmoj]

Then in order to keep the best view and consistent png results we could default layout.scene.bgcolor to white instead of rgba(0,0,0,0).

[archmoj]

@etpinard all the tests passed and this PR should be ready to merge.

[etpinard]

Can I ask why you're now passing these options from plotly.js? The corresponding option object current on master is:

    var glplotOptions = {
        canvas: canvas,
        gl: gl,
        container: scene.container,
        axes: scene.axesOptions,
        spikes: scene.spikeOptions,
        pickRadius: 10,
        snapToData: true,
        autoScale: true,
        autoBounds: false,
        cameraObject: cameraObject,
        pixelRatio: pixelRatio
    };

that is, the glOptions container is new.

[archmoj]

Where is that set on master?

[etpinard]

plotly.js/src/plots/gl3d/scene.js

Lines 201 to 213 in 8122c21

	var glplotOptions = {
	canvas: canvas,
	gl: gl,
	container: scene.container,
	axes: scene.axesOptions,
	spikes: scene.spikeOptions,
	pickRadius: 10,
	snapToData: true,
	autoScale: true,
	autoBounds: false,
	cameraObject: cameraObject,
	pixelRatio: pixelRatio
	};

[archmoj]

https://github.com/gl-vis/gl-plot3d/blob/d206f1f110d427b2baae0462964bc59cf7f59dae/scene.js#L80-L87
The glOptions used to be left undefined and hidden to plolty.js developers.
Now that are explicitly defined, it should be easier to figure out what the differences between static & interactive plots are.

[etpinard]

Ok, that's fair. But then is-mobile becomes a top-level dependency for plotly.js and we should add it to our package.json.

[archmoj]

Good catch. OK if I add it to package.json?

[etpinard]

OK if I add it to package.json

yes please!

[archmoj]

Simply listed in ff2ecc7.
But also noticed that it was added in package-lock before which is not clear why?
75de9e5dea

[etpinard]

But also noticed that it was added in package-lock before which is not clear why?

Yeah it was in the package-lock before, because the package-lock lists all the dependencies not just the top-level ones. It's a best practice to include all the top-level dependencies in the package.json, and make npm handles the nested dependencies part in the package-lock.

[etpinard]

💃 - nicely done!

Before merging, could you change the PR title to something like "Refactor gl3d scene" as this PR no longer disables premultipliedAlpha.