Triangle rendering fixes #3472
Triangle rendering fixes #3472
Conversation
- fixed FLX_RENDER_TRIANGLE not rendering anything - fixed FlxCamera.drawTtriangles not accounting for color transformation on FlxG.renderBlit - removed flash conditionals in FlxStip, FlxCamera and FlxDrawTrianglesItem - deprecated "colors" in FlxStrip - deprecated "colors", "verticesPosition", "indicesPosition" and "colorsPosition" in FlxDrawTrianglesItem
| @@ -835,7 +835,7 @@ class FlxCamera extends FlxBasic | |||
| position = renderPoint.set(); | |||
|
|
|||
| drawVertices.length = 0; | |||
| final verticesLength = vertices.length; | |||
| final verticesLength = Std.int(vertices.length / 2) * 2; | |||
There was a problem hiding this comment.
Do you know of a case where we would need to round to the nearest multiple? is it possible to add an incorrect multiple? if so we shouldn't allow that, assume it was a mistake and throw an error then, rather than accounting for it everywhere.
Also, are you sure that in the case where vertices is not the correct length, will this code not malfunction? I imagine it would hit an out of range error
This goes for all cases
|
Also Making 8 sweeping changes in one PR is often 16 times more work for us to verify rather than making separate PRs for each change. This is huge and there's a ton of code style changes adding noise and I don't really know how to test all this |
There was a problem hiding this comment.
I would close this and try again. Any feature/fix in here that does not have an individual issue, please make one and discuss the need for this change (please include examples). There's a ton of changes here that I'm not a fan of, as well as some general code style/refactoring and you mention "optimizations", all of these are making it really difficult to determine whats actually needed, and why.
- FlxCamera.drawTriangles now correctly accounts for camera bounds.
Make an issue with a snippet that reproduces this problem.
- FlxCamera.drawTriangles now accounts for color transformation on FlxG.renderBlit.
This needs as issue as well, if one doesn't exist (it might). I see a lot of #if flash taken out but Note that renderBlit may be true for targets other than flash. This also needs an reproducing snippet
| @@ -571,16 +571,15 @@ class FlxCamera extends FlxBasic | |||
| static var renderPoint:FlxPoint = FlxPoint.get(); | |||
|
|
|||
| static var renderRect:FlxRect = FlxRect.get(); | |||
|
|
|||
| @:noCompletion | |||
There was a problem hiding this comment.
These shouldn't be exposed by default, I think they'll just confuse 99% of devs and should be reserved for internal use or people who know what they are messing with, eventually we should try and separate the "backend" rendering features from the front-end camera object functionality, but for now, let's leave this
| // TODO: catch this error when the dev actually messes up, not in the draw phase | ||
| if (graphic.isDestroyed) | ||
| throw 'Cannot queue ${graphic.key}. This sprite was destroyed.'; | ||
|
|
There was a problem hiding this comment.
Can you share a case that this guards against? Since this is function is supposed to be used internally I imagine there's a better place to check this
| @@ -663,6 +663,9 @@ class FlxFrame implements IFlxDestroyable | |||
| cacheFrameMatrix(); | |||
| } | |||
|
|
|||
| // update uv | |||
| frame = frame; | |||
There was a problem hiding this comment.
In general, self referential statements like this should be avoided. Stuff like this make me regret adding getters/setters
| var rect = frame.frame; | ||
| rects.push(rect.x); | ||
| rects.push(rect.y); | ||
| rects.push(rect.width); | ||
| rects.push(rect.height); | ||
|
|
||
| rects.push(frame.frame.x); | ||
| rects.push(frame.frame.y); | ||
| rects.push(frame.frame.width); | ||
| rects.push(frame.frame.height); |
|
Closing this, need to rethink changes that i made. |
Changes list:
FLX_RENDER_TRIANGLEnow rendering correctly. (DrawTriangle's rendering broken, not rendering #3169)rightandtopvalues no longer swapped inFlxUVRect. (FlxFrame: Incorrect UV values #3471)FlxStrip.colorsnow reimplemented. (colors don't work in FlxStrip #2263)FlxFrame.clipnow updatesuv.FlxCamera.drawTrianglesnow accounts for color transformation onFlxG.renderBlit.FlxCamera.drawTrianglesnow correctly accounts for camera bounds.colors,verticesPosition,indicesPositionandcolorsPositioninFlxDrawTrianglesItem.Main changes (
FlxDrawTrianglesItem)colorsvector is a part ofopenfl-legacyGraphics API that Flixel used to apply color transformation to traingles onFlxG.renderTile. Right now coloring onFlxG.renderTileis handled by shader, butaddTrianglesandaddQuadhasn't been updated for this, resulting inaddTrianglesjust "ignoring"colors(#2263) andaddQuad(used forFLX_RENDER_TRIANGLE) being completely broken (#3169). Both functions were updated to fix respective issues:addTrianglesnow accounts forcolorsalsongside regular color transformation (Note: inflixel-addonssome classes still usecolorsthe old way, so this will need a fix pr when merged) andaddQuadnow properly stores color transformation.Also deprecated
colorsandcolorsPositionfor not being used andverticesPositionandindicesPositionfor being pretty much pointless.TODO (?)
FlxStrip.repeatsinceFlxDrawTrianglesItemjust ignores it and usesREPEATfor shader anyway. 🤷♂️🤷♂️NOTE
Turns out
camera.alphadoesn't affect transparency of triangles due to openfl bug. Might need workaround for that until it's fixed.Update: in progress of being fixed. Thanks Redar! (openfl/openfl#2799)