ObjectContainer breaks when position is minus infinity
When I set position to -Infinity and width to Infinity there are multiple problems that prevent a container from being drawn.
-
camera.isVisiblewill always return false becauserightandbottomare NaN (-Infinity+Infinity). https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/shapes/rectangle.js#L136 and https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/shapes/rectangle.js#L154 - this could be fixed by checking if the width/height are Infinity. In that case the position should be ignored.
-
globalTranslationis translated byobj.posand afterwards moved in the reverse direction which again produces NaN and breaks more stuff asglobalTranslationwon't be reset to a valid number. https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/renderable/container.js#L723- saving the
globalTranslationposition at the beginning of theupdatemethod and resetting it to that value would fix this.
- saving the
var isTranslated;
var x = globalTranslation.pos.x;
var y = globalTranslation.pos.y;
var viewport = me.game.viewport;
...
if (isTranslated) {
globalTranslation.pos.set(x, y);
}
- everything inside the container gets translated by
-Infinityand thus won't show on the screen. https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/renderable/container.js#L761- This could be avoided if the container translates its children respecting the anchor point. In cases where any of the bounds are Infinite, the anchor point should be at 0 in that axis in order to prevent children from being moved to Infinity.
The first two points should not pose a problem, but the third one will break pretty much everything, unless ObjectContainer sets the anchorPoint to [0,0].
If you are okay with the proposed solution, I can create a pull request.
It seems these infinite planes are completely unnecessary, to be honest.
- An infinite plane is always visible within the viewport -- Easy solution; don't check.
- A child cannot be relative to an infinite plane's upper-left corner.
For the first point, I'm experimenting with removing inViewport checks entirely, using a better data structure for tracking children in an ObjectContainer (removing objects from large arrays is very slow.) That will take care of the first issue.
The second point is more difficult than setting an anchorPoint (which defaults to 0,0 anyway). The anchorPoint is just a normalized vector, e.g. 0.5,0.5 is the center point. The result vector is calculated by rect.width * anchorPoint.x, rect.height * anchorPoint.y; Infinity * 0 is NaN.
I agree that most games won't ever need this, but in my game the camera can move freely in every direction. This would still not be a problem as long as I have all renderables inside the rootContainer, but because I want to implement a zoom feature where different layers scale based on their z-distance, I need such an infinite plane (for the layer).
If you think this is not an issue with melonJS I can also close this ticket and just copy the container class and make my own adjustments. But the fact that the implementation breaks in some corner cases remains ;)
What I meant with "the anchor point should be at 0 in that axis" is that in order to avoid Infinity*0 there would have to be an additional check for width=Infinity.
Don't get me wrong, infinite scrolling provides an excellent user experience
I just think we should be careful about trying to make infinite planes that act like finite planes. In other words, we should support "infinite scrolling" and such, but probably as a first-class object that doesn't necessarily have the same logic flow as finite planes.
Or we could just use a different coordinate system! If an infinite plane is given a logical origin at 0,0 which all children are positioned in relation to, the origin of the plane can be translated just like the position of a rectangle is translated today.
Just some ideas to consider; I don't know at this point if it would also be advantageous to give all shapes an "origin". I've been under the impression that shapes need something like an origin for a while, but right now we are kind of flipping it on its head to treat the anchorPoint as an origin.
See #303, #366 for a reference description of how a rectangle should be defined. Position is relative to its parent, origin is implicitly 0,0, and its vertices surround the origin. In the case of an infinite plane as a direct descendant in the world container, its position will always be relative to the viewport, and there's no need to translate its position to the upper-left corner.
Boundary checking becomes easier, since the left, right, top, and bottom properties are directly related to the shape vertices; not a sum of position and size. And translating children from it's parent origin (parent's absolute position) is simple without introducing NaN, because all origins are zero-based, not upper-left-corner-based.
The infinite plane was just an idea I had yesterday after not being able to think right anymore :P A better way to achieve what I want is to do as you say and handle the logic differently. For now I will make the container float and overwrite the draw method to not do any translations.
@insidiator Don't worry, I'm still fighting to win the shapes battle. It's a hard one, it seems. :) But it's literally the only way we can do 2D rendering in a variety of configurations while keeping the math sane.
ahah, the shapes battle :P
moving to 1.2.0
For infinite planes, we need to rework our entire architecture; everything needs to be relative to an arbitrary point of origin, instead of upper-left-corner.
with the latest version of Tiled supporting Infinite world, that would be nice to have this one working properly !
When I set position to -Infinity and width to Infinity there are multiple problems that prevent a container from being drawn.
camera.isVisiblewill always return false becauserightandbottomare NaN (-Infinity+Infinity). https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/shapes/rectangle.js#L136 and https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/shapes/rectangle.js#L154this could be fixed by checking if the width/height are Infinity. In that case the position should be ignored.
globalTranslationis translated byobj.posand afterwards moved in the reverse direction which again produces NaN and breaks more stuff asglobalTranslationwon't be reset to a valid number. https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/renderable/container.js#L723
- saving the
globalTranslationposition at the beginning of theupdatemethod and resetting it to that value would fix this.var isTranslated; var x = globalTranslation.pos.x; var y = globalTranslation.pos.y; var viewport = me.game.viewport; ... if (isTranslated) { globalTranslation.pos.set(x, y); }
everything inside the container gets translated by
-Infinityand thus won't show on the screen. https://github.com/melonjs/melonJS/blob/1.0.0-dev/src/renderable/container.js#L761
- This could be avoided if the container translates its children respecting the anchor point. In cases where any of the bounds are Infinite, the anchor point should be at 0 in that axis in order to prevent children from being moved to Infinity.
The first two points should not pose a problem, but the third one will break pretty much everything, unless
ObjectContainersets theanchorPointto [0,0]. If you are okay with the proposed solution, I can create a pull request.![]()
All committers have signed the CLA.![]()
All committers have signed the CLA. _Originally posted by @CLAassistant in https://github.com/n8n-io/n8n/issues/3437#issuecomment->Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it. https://winksite.com/site/resources/banner-image-template.png
Uploading export-FW4248-1654966774.csv…
closing this one, see #1122