Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change draw to take some kind of DrawContext data #2715

Open
Geokureli opened this issue Jan 17, 2023 · 5 comments
Open

Change draw to take some kind of DrawContext data #2715

Geokureli opened this issue Jan 17, 2023 · 5 comments

Comments

@Geokureli
Copy link
Member

Geokureli commented Jan 17, 2023

Been thinking on this for a while. We could remove FlxSpriteGroup if the draw() call stack took some kind of matrix or rendering context from their parent group or w/e.

The basic idea

create a DrawContext class with a matrix, and any rendering/color info that you want to bubble down to members. So, calling draw on any container/group will cause it to copy the incoming draw context and augment it with it's own orientation and rendering properties, which it will pass down to it's members. Since FlxGroups do not have orientation or color info, they simply pass down the context they received, verbatim, but FlxNests (Or whatever replaces FlxSpriteGroups) will add their info to the context before passing it down.

Possible fields, these would either be compounded with the parents, or a member might override them if it has a unique or non-null value:

  • matrix/positioning - explained above
  • cameras - just like the current system, a member with null cameras will use the parent's cameras
  • color/shaders/rendering - shaders would likely behave like cameras, where members' shaders override the parent's default, but color can combine, maybe?
  • clipRect - a members' actual clipRect would be the union of both it's own rect and the one passed from the parent

Advantages

Not only will this simplify "nesting" over FlxSpriteGroups, which bubble changes down every time they are changed (creating a ton of caveats and confusion), FlxNexts don't need to be limited to sprite members, you can add basic FlxGroups.

We could also give camera's a DrawContext, or its own orientation and color rendering information, that is applied to all sprites rendered to it, allowing cameras to have rotation (related to #2679)

We could stop setting FlxCamera.defaultCameras in FlxGroup. DrawContext would allow groups to pass their desired cameras down to members.

Drawbacks

The main drawback of this is that you can't perform collision checks between objects in different nests and expect them to collide in regards to their rendered positions, as it will collide them in their respective local positions. This won't effect FlxGroups since their members use the same coordinate space as the group. but the new alternative to FlxSpriteGroups will have this issue

@Geokureli
Copy link
Member Author

Geokureli commented Feb 16, 2023

One huge drawback I'm not sure I can overcome is mouse events. if the button is placed locally at, say (0, 0) but the parent is at (100, 100), how do we check for mouse overlaps at it's global position rather than it's nested postion? All of my ideas have issues:

  • check for mouse overlap in the draw phase
    • breaks convention, draw phase should be used for rendering only
    • doesn't address the fundamental issue with getting an object's screen position
  • store the DrawContext used on the previous draw
    • hacky
    • doesn't work with sprites drawn to multiple cameras
    • won't know if the parent has moved until the next draw call
  • store a ref to the parent
    • openfl does this, but displayObjects can only have a single parent, flxBasics can be in many groups
    • Even if we allowed a sprite to be in multiple groups but only one "nest", this would mean devs can't utilize draw context in their own containers (can provide examples)

note this isn't just an issue with mouse overlaps, it generally causes issues in knowing the screen position of anything. Additionally, solving this could solve the issue of colliding objects in different nests with different positions, though that isn't as high of a priority.

IMO this is pretty damning for this feature unless a solution is found

@Geokureli
Copy link
Member Author

I think an easy way to add a refs to each members parents is to create a FlxContainer that extends FlxGroup, add a parent field to FlxBasic and only allow them to be in one container, by removing them from their previous parent on add. FlxState should extend FlxContainer instead of FlxGroup

@Geokureli
Copy link
Member Author

Moving to 7.0.0, 6.0.0 is already getting pretty hefty, and I don't want too many breaking changes at once. might still add it to 6.0.0 if things go well but this is likely gonna hold up 6 too much

@01010111
Copy link
Contributor

The way I've seen some other frameworks handle this is by extensively using global position for everything, for example: https://pixijs.download/v4.8.9/docs/PIXI.DisplayObject.html#getGlobalPosition

@Geokureli
Copy link
Member Author

The way I've seen some other frameworks handle this is by extensively using global position for everything, for example: https://pixijs.download/v4.8.9/docs/PIXI.DisplayObject.html#getGlobalPosition

getGlobalPosition/Bounds is planned, now that objects have a ref to their parent we can calculate it by bubbling up the display tree, as for actually drawing them, though it seems wasteful to calculate this for every object, every draw frame, that's where drawContext comes into play, its a way pass the precalculated matrix (and other drawing data) down to members, so they can calculate their global position without doing it all from scratch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants