Conversation
doesn't seem to be a reason for them to be in FlxCameraView? if I've overlooked something then we can easily move them back at some later point in time
|
I decided to deprecate all the internals and make them point to where they were moved, to avoid breaking anything. Addons and UI will need to be updated to avoid warnings, I'll get to that in a bit. Addons specifically seems to have an issue because I think this is in a good enough of a state now for a review, so I'll undraft |
|
Some more notes and thoughts. I'm currently playing around trying to implement an OpenGL renderer to really test the flexibility of this system. Overloading methods in FlxCameraView won't work:For stuff like FlxMaterial and FlxTrianglesData, the function signatures of the core rendering functions need to be changed. To avoid breaking changes, I was just going to do this with overloads, but that didn't work out because overloads need to be inlined and you can't override inline functions. I got around this by deprecating drawPixels() and copyPixels() for draw() and copy() in FlxCameraView, but I couldn't do the same in FlxCamera because it inherits FlxBasic.draw(). I ended up overloading the camera's drawPixels() and copyPixels() to call the new methods in camera view. Pretty gross solution IMO but I can't think of a better way without a breaking change. Unifying renderBlit with other renderersI did some work related to this in 3cd97d9, but there's still a couple places where we have to do things differently depending on the renderer, notably Lines 743 to 744 in c32ab91 When the blitting renderer is used, Isolating direct access to renderer
EDIT 2: This is no longer an issue with recent changes, see next comment |
WIP; debug drawing is not functional yet and there's a bunch of temporary code that needs to be cleaned up
|
Originally this was just a port of the renderer abstraction by Beeblerox that I ported over to modern Flixel. As I was playing around with it, I found that there were certain things I wasn't too fond of, specifically the fact that access to the renderer was only possible through cameras, so here goes an attempt at a V2.
Considering that the renderer is now global, I've also integrated some of the helpers mentioned in #3527 directly into I also wonder if it'd be worth deprecating the drawing methods in |
|
I think this is now in a reviewable state. No clue why CI is failing tho |
|
Apparently |
|
Nice work, thanks for helping out with this. I like most of the changes here, though I do have some concerns/questions. Re: No.3I think moving
I don't think I completely understand what you mean. If you mean that Re: Decouple FlxCameraView from FlxCameraI've been thinking about this too. I think it's doable to have some arbitrary buffer draw commands can be executed on (though perhaps this should just be Food for thought, Beeblerox's Re: Obfuscate openfl.display.GraphicsHmm, some food for thought, the renderer overhaul had the Re: "Quad" vs "Tile"This bothered me as well, but I couldn't decide what to do. Personal concernsBreaking core function signaturesThis is not completely related to these specific changes, but I thought I'd mention it so that we could future proof stuff before things are set in stone. Ideally I was hoping that the OpenGL renderer, which I'd implement some time after this is merged, wouldn't break anything. That way it could be locked behind a compiler flag in Flixel 6.x while it gets fully polished and tested, and made default in Flixel 7. (and perhaps we drop the One issue with this is that the renderer overhaul comes with some function signature changes. As an example, here's what // Old
function drawTriangles(graphic:FlxGraphic, vertices:DrawData<Float>, indices:DrawData<Int>, uvtData:DrawData<Float>,
?colors:DrawData<Int>, ?position:FlxPoint, ?blend:BlendMode, repeat:Bool = false, smoothing:Bool = false, ?transform:ColorTransform,
?shader:FlxShader):Void {}
// New
function drawTriangles(graphic:FlxGraphic, data:FlxTrianglesData, material:FlxMaterial, matrix:FlxMatrix, ?transform:ColorTransform):Void {}It's a pretty significant, and IMO welcome change. I've been thinking over how to implement it in a non-breaking way though. Everything I've thought of thus far has some flaw:
It's not really crucial that the OpenGL renderer gets into Flixel 6, but I think it'd be really nice, and also would make the transitional period a lot smoother. |
The secret is it's always gonna be ugly. I tend to make new methods, in your example I'd add drawMaterialTriangles or something.
The sooner we try to utilize the new system the sooner we'll realize we're doing something wrong. The more we can plan now, the better.
This is where I'm really struggling to understand the idea. This isn't really how I imagine other global utils are used, and it seems weird, here. Is the idea behind FlxG.renderer that the "screen" is a global object that things draw to, where the views simply determine how world coordinate translate to screen coordinates? I see it very differently, where the screen has a bunch of render targets, with specific layering, orientation, filters and properties. To me objects draw to a render target, regardless of the backend, even in the case of a GL backend, the Flixel dev is choosing which sprites draw to which render target. Perhaps the backend combines that all into one actual screen, but the notion of these being individual things is still important. Thats why filling a view instance via a global call feels weird. In contrast, say I wanted to fill the screen at an arbitrary time during the draw call, say this had nothing do with a specific view, I simply want the entire screen filled, then I could see using
Kinda, maybe, idunno...? In the example above fill wouldn't be private, and it's public in your gl example, so that confused me. I also see that your glRenderer's fill uses a shared rect, so the setup kinda makes sense, unlike the blit and quad views which each have a separate displayObject they draw to, gl can just use the same one. To me though, it would make more sense to put a global I think regardless of what's going on in the backend, object's specified to draw to render targets should call methods on that render target to draw itself. The global util should not need to be an intermediary between those (at least as the default way of drawing to a view). FlxG.renderer should more be for things like project wide settings/configs, and global tools that interact specifically and directly with the backend renderer. I hope this makes sense, I did the thing where I type all this right before going to bed. I have no concerns or comments on any of your other points |
I'll mostly talk about the potential OpenGL renderer here, since it follows the traditional render setup, though the same logic should apply to the other render backends. The latter interpretation is correct. There can and will be multiple different render targets that all the drawing commands will be executed on. This is how cameras will work, with each camera having its own render texture. The only interaction with the screen (back buffer) will be at the end of the frame, when the camera textures are drawn. My goal with the global renderer was to keep all the core backend logic centralized, so that we'd keep all the backend OpenGL calls in one place, rather than scattered across multiple classes. There's also benefits to keeping some stuff, like vertex buffers global on a renderer-level rather than per render-target. For example, to batch sprites, we really only need one vertex and index buffer as any state change (such as a render-target swap) will require us to do a draw call and flush the buffer. If we sort our draw commands* to be in order per render target, we can draw everything without a buffer change, which is good because in OpenGL you should keep state changes to a minimum (for reference in the link, a VBO refers to a vertex buffer). *Stuff like this is why I was considering introducing some sort of draw command buffer, ideally without any additional overhead. I believe modern graphics APIs (i.e. Vulkan, Metal, WebGPU) also use command buffers, so I guess that's future proofing in a way. We could add convenience methods per view that are basically just something like // in FlxCameraView
public inline function drawPixels(...)
{
FlxG.renderer.drawPixels(this, ...);
}but I still think that all the main rendering stuff should be handled by
We could allow direct drawing to the screen (back buffer) if the passed in
I think I cleared up the points here in the paragraph above
Hmm, though |
|
I think my issue is with enforcing the same API across all backends. I'm okay with low-level helpers being in an extension of renderer, but having all these methods defined in the base renderer, and redefined in extensions is going to make things overly rigid, and not well organized in most cases. The interface with the render targets doesn't need to match the interface with the global renderer and if we break that connection I imagine things will go much more smoothly.
I kinda (mostly) agree with this. I think var renderer(get, never):FlxGLRenderer;
inline function get_renderer() return cast (FlxG.renderer, FlxGLRenderer);
override function fill(color:FlxColor, blendAlpha:Bool)
{
renderer.fillRect(getScreenRect(), color, blendAlpha);
}This is somewhat pseudo code, I dunno if renderer.fillRect draws to the default buffer or whatever, but note that That said, while all renderers don't need to define the same helpers/tools, we may want to define common tools in FlxG.renderer that people can access without caring about what backend is being used, but those global fields do not need to match view's common methods. Things like FlxG.renderer.drawMode = PIXEL_PERFECT |
|
Yeah I think this could make sense. I don't know how I feel about implementing draw methods into I suppose what could really help clear things up is to actually try and implement the GL renderer into the new abstraction, which is what I'll be doing soon. Is your branch up to date? Not gonna merge here yet, but just so I know for reference locally |
|
Gonna try a couple things, I'll let you know when I'm done, hopefully by sometime tomorrow |
Co-authored-by: ACrazyTown <47027981+ACrazyTown@users.noreply.github.com>
|
quoted from #3567
The important distinction I wanna make is that, while we may move the rendering somewhere else, I do think it's important that to draw a sprite to a camera's view, you should call something akin to
The reason I make FlxVertexBuffer an abstract, is so that method args could be changed to take that type without breaking existing calls to it (as Graphics and FlxVertexBuffer can be implicitly casted to one another). Once devs migrate, FlxVertexBuffer will be a different type that will allow (or wrap) various underlying types. 1 option is to make it a class wrapper with derived types that use each specific underlying type. Another option is an abstract OneOfMany<T...> that calls the appropriate renderer |
Might rename again in the future to something more debug oriented...?
Yeah this makes sense. To be clear, I'm not against putting the render methods in camera view, I'm moreso against tying them to cameras specifically, as this would lead us further away from separating cameras and rendering (#1073). It'll be easier to reason with this once we have all the missing pieces, so my focus now is to implement In the meantime, I'd appreciate any help on #3566, as I think that's going to be one of the toughest parts to figure out.
I don't think I can make
Hooking it up is doable, it's just an extreme hassle because everything expects a |
Was causing crashes with the GL renderer as it doesn't use the FlxDrawItems at all. Seems like there's no functional difference between manually adding quads to a batch and just calling drawFrame() and having it done automatically.
Here we go, first step towards a renderer overhaul. Looks like a pretty big and scary change but 99% of this is just moving stuff around.
Shout out goes to Beeblerox & Austin East, a lot of this is based on the original renderer overhaul branch, I'm just porting bits and pieces to modern Flixel.
Changes
FlxRenderer
Adds a base
FlxRendererclass, accessible via the globalFlxG.renderer, which serves as the base for all rendering functionality. Renderer implementations extend it and implement the required methods.The blitting and draw quads renderers have been ported to
FlxBlitRendererandFlxQuadRenderer, respectively.Since the renderer is a global thing, and not per-camera anymore, it works slightly differently. Before any calls to drawing commands you need to call
FlxG.renderer.begin(camera);. This will be done internally by Flixel during a sprite's draw phase, but it's something to keep in mind if you're doing something out of the ordinary!FlxCameraView
Adds a base
FlxCameraViewclass. Like with renderers, different implementations extend the base class and add onto it. Camera views mainly just hold per-camera rendering related objects, stuff like OpenFL sprites and whatnot.To avoid breaking changes, you can get a typed reference to the camera view using the
camera.viewBlitandcamera.viewQuadshortcuts. Use this to reference stuff likecamera.flashSpriteorcamera.canvas.Other previously established stuff
Most rendering methods from
FlxCamerahave been deprecated. If you need to issue drawing commands manually, use theFlxG.rendererAPI instead.I say most because some batch related methods (e.g.
startQuadBatch()) have been left as-is. I plan to tackle these at a later point and in a different PR.I'm opening this as a DRAFT, because:
Would be nice to add docs to a bunch of stuffI don't know what to do with a bunch of private internal variables in FlxCamera and alike. What's Flixel's way of handling this? Should I simply get rid of them, or deprecate them and make them point to where they were moved?TODO: