Closed w0rm closed 7 years ago
Thanks for reopening this here @w0rm.
Any chance of merging this anytime soon?
@Zinggi I really want this feature myself. However any change that is minor or major should be approved by Evan.
I understand this restriction. I'm wondering whether anyone has approached Evan about this.
Also, maybe he has changed his opinion, it has been a year since that comment:
I don't really know who is part of elm-community or what the goals are.
Is probably no longer true.
@Zinggi I don't think anyone has directly approached Evan regarding these changes. But if we want to make a major change, then its better to see what else can we bake into the release and give enough information to Evan about these features and their use cases.
Alright. Should we open a meta issue to collect use cases and to group changes?
@Zinggi sure! From what I see, #6 may be merged as a patch change, and #5 is blocked by #4.
@Zinggi I wonder what will happen if we render with one set of options, and then change them. I think it needs to recreate gl
. But this case is not handled at the moment.
You're right, that is not handled.
I gave it a try and at least it doesn't crash or anything, but it obviously doesn't work as would be desired.
I tried changing alpha
at runtime (as this is visible immediately) but nothing happened.
So it's not too bad.
Also, the only option that makes sense to change at runtime is antialias
.
After playing around with it a bit, I managed to find a workaround to toggle these parameters in elm.
Basically just create two different elements using WebGL.toHtmlWithEvenMore
, one with antialias = True
, the other with antialias = False
. Then render both to the dom, but the one with the configuration you don't want can be hidden by setting width and height to 0 and not actually rendering anything (so that you don't do useless calculations). Like this:
renderWorld doShow entities =
let
( w, h, objects ) =
if doShow then
( size.width, size.height, entities )
else
( 0, 0, [] )
in
WebGL.toHtmlWithEvenMore { defaultContextAttributes | alpha = doShow }
defaultConfiguration
[ width w, height h, style [ ( "display", "block" ) ] ]
objects
and use it like this:
div []
[ renderWorld useAlpha entities
, renderWorld (not useAlpha) entities
This is not very nice, but at least it works.
Given that this is not even possible in three.js (if you trust this SO answer), I don't think this problem should block this pull request. But we might find a better solution later.
@Zinggi I think we can do better and maybe create the new gl
context when these settings differ.
I agree, we can certainly do better.
A nice way of solving this would be to used a keyd node, with a key based on the settings.
But I don't know enough about the virtual dom library to do that. There seems to be no customKeyedNode
.
Can you think of a better solution?
@Zinggi we can check it in the diffing function https://github.com/elm-community/webgl/blob/d36041d6d1008bcea406ad62c2accf7a68324fc3/src/Native/WebGL.js#L598 and if this changed, then we recreate all things in model.cache
actually there are already function calls in render that will only be applied once, even if they change
I gave it a quick try but failed to get it working.
It's not as simple as just recreating the cache. I think you have to actually destroy the canvas element and recreate it. But I don't know how to do that correctly with virtual dom.
But I'll keep trying
@Zinggi I've found someone doing this: https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_lose_context#Examples
we can also try creating and returning a different canvas node from applyPatch
, looks that it may actually replace the old node
Today I tried various different ways to let the canvas node be replaced by virtual dom, but I couldn't get anything to work.
So I tried another way. My solution might be a bit hacky, but at least it works. This is how I do it:
function diff(oldModel, newModel) {
newModel.model.cache = oldModel.model.cache;
if (oldModel.model.contextAttributes.antialias !== newModel.model.contextAttributes.antialias) {
var canvas = newModel.model.cache.gl.canvas;
var p = canvas.parentNode;
// clear webgl context
var ext = newModel.model.cache.gl.getExtension('WEBGL_lose_context');
ext && ext.loseContext();
// clear cache
newModel.model.cache = {};
var newCanvas = renderCanvas(newModel.model);
// copy old attributes to new:
var attributes = Array.prototype.slice.call(canvas.attributes);
while (attributes.length > 0) {
var attr = attributes.pop();
newCanvas.setAttribute(attr.nodeName, attr.value);
}
// and replace the old canvas with the new one
p.replaceChild(newCanvas, canvas);
}
return {
applyPatch: drawGL,
data: newModel
};
}
I deliberately only compare the antialias attribute, as I think this is the only one that makes sense to be toggled at runtime.
How can I update this pull request? Usually I could just push to my fork, but since this is no longer a fork of mine I don't know. Should I create a new pull request with my changes? Or do you know how I can push to this pull request?
As discussed privately, this 'solution' is too hacky. It's better to not allow modifying these attributes at runtime than to implement this in a hacky way. If someone comes up with a legitimate use case that cannot be solved in another way we can reconsider.
Replaying the original pull request by @Zinggi on top of the current master.
I've only squashed commits and rebased the branch.
This pull request addresses the following issues:
Quoting the original pull request: