Double memory in FlxSprite

[MSGhero]

Whenever FlxSprite functions (makeGraphic(), loadGraphic(), set_pixels()) are called for the first time, the input/created bitmapdata is always duplicated. One gets stored in frames[0], and another is created by framePixels = frame.paint(framePixels, ...) where parameter framePixels is null. This is fine if I have multiple frames, but for a static background image, I only need one copy of the image. This is a problem when I have 64 bg images that take up 36 MB alone, duplicating takes up 72 MB for no reason.

Is there any way to skip set_frames()/getFlxFrameBitmapData() while keeping FLX_RENDER_TILE enabled? Or would this require a new class?

[larsiusprime]

To be clear, this happens on all targets, not just flash?

[larsiusprime]

Just on a cursory check, this MIGHT be what FlxBackdrop is intended for: https://github.com/HaxeFlixel/flixel-addons/blob/6cb954046348335d89da2096f9498cec3ca5f921/flixel/addons/display/FlxBackdrop.hx

[MSGhero]

I've never seen that class before. I can't compile my project to anything other than flash right now, but yeah it should be every target when FLX_RENDER_TILE is active.

[Tiago-Ling]

I use FlxBackdrop in almost all my projects. Essentially it is a repeating background image. You can set it's vertical and/or horizontal movement speed relative to that of the camera. Very useful for parallax backgrounds and afaik works on all targets.

Tiago Ling Alexandre Tel: +55 41 8819-3191

2015-08-28 17:53 GMT-03:00 Nick notifications@github.com:

I've never seen that class before. I can't compile my project to anything other than flash right now, but yeah it should be every target when FLX_RENDER_TILE is active.

— Reply to this email directly or view it on GitHub https://github.com/HaxeFlixel/flixel/issues/1593#issuecomment-135885394.

[MSGhero]

My backgrounds don't parallax or scroll or anything, but I'll give it a try.

[MSGhero]

Lol no. It triples rather than doubles bmd memory allocs, but then 1/3 are successfully disposed of according to Scout. So basically nothing changes. The issue with backdrop is that it too calls makeGraphic() which calls set_frames() which calls getFlxFrameBitmapData() which creates a new bitmapdata.

Basically, I want to set framePixels directly, ignoring everything else in FlxSprite, but setting it directly doesn't do anything since it's a variable.

[larsiusprime]

Okay, so it looks like we might need to extend or modify a class for this? Either FlxBackdrop or FlxSprite itself?

[Tiago-Ling]

Loading FlxSprite frame directly is possible using an FlxAtlasFrames object, that's what i'm doing currently. I do not know if it'll change allocations, but it's working for me in cpp using the following code:

//Atlas Frames
var atlasFrames = FlxAtlasFrames.fromTexturePackerJson(atlas_gfx.png', 'atlas_data.json');

//Single Frame
var spr = new FlxSprite(0, 0);
spr.frame = atlasFrames.getByName('frame_inside_atlas.png');

//Multi frame (animation)
var anim = new FlxSprite(0, 0);
anim.frames = FlxTileFrames.fromFrame(atlasFrames.getByName('tileset.png'), FlxPoint.weak(0, 0));

Do not know if this approach is doable in Flash though.

[larsiusprime]

Neat, thanks for that.

[Tiago-Ling]

By the way, i'm using it with the TexturePacker tool, with the export option "json array". This works perfectly with HaxeFlixel with no other adjustments being necessary. All my atlases use this format and so far it works fine.

One thing though: you may want to leave a 1px border around all atlas objects as it can prevent tearing in the images.

[MSGhero]

I guess I'll give more details now. I have a spritesheet (atlas frames), and I'm painting individual sprites as needed to 64 raw bitmapdatas that are 516x288 each. Then I create a background FlxSprite to set each bmd to in order to display them. What I really want is a flixel version of flash.display.Bitmap with framePixels's object reference exactly equal to the bitmapdata that I created earlier. This seems to be relevant to non-animated sprites only.

(I can probably bring the count down to 5 at a time fully rendered versus 64, but still)

My spritesheets are made in TexturePacker too, best tool I've used for packing.

[larsiusprime]

I guess we just need to go ahead and create a FlxBackgroundSprite or something, that is optimized for this use case.

Additionally, it could be set to a single color instead of a sprite, and instead of allocating a whole bunch of pixels, just create a 1x1 pixel and hardware scale it. This would make it a really memory efficient choice.

[MSGhero]

This sorta brings FlxBaseSprite back to mind from a while ago... would BG sprite be a separate entity, or would FlxSprite inherit from it?

[Tiago-Ling]

In that case i'd say you could simply use a flash.display.Bitmap directly and add it under the mouse cursor?

This relates to an old discussion about having an intermediary class between FlxObject and FlxSprite - in this case a simple FlxImage class or something, or even using composition and moving the visual aspect of FlxSprite into another class. The overall consensus was to not do it because it would break everything.

[larsiusprime]

I think an extended FlxSprite that just overrides the double-bitmap behavior, without trying to be some new fundamental type that shakes the existing architecture loose, would do the trick.

Using a Bitmap directly could work, but what if you want it in the middle or back of your flixel display list?

[MSGhero]

^ The background sprite is within a sprite group also containing the foreground so that I can tween them together for map transitions.

[Tiago-Ling]

I see - in any case, this bitmap copying behavior is far from ideal and in my opinion should be fixed if possible.

By the way @MSGhero, is there any difference if you just set the frame or frames instead of using the BitmapData directly (i.e. framePixels) ?

[larsiusprime]

This does bring up a related note -- on native, now that OpenFL is using cairo to rasterize text fields, OpenFL is holding on to an already extant rasterization of the text field, but flixel is still using the old behavior and generating (and storing) its own copy. So we have double memory use on text fields, too. But that's probably for another issue...

[MSGhero]

@Tiago-Ling I'm setting pixels directly since that has a setter, setting framePixels directly doesn't actually do anything/it gets overwritten since frames is null. set_pixels() eventually calls set_frames(), so the same thing would happen if I only set frames. I'm not sure if it's optimal/required, but it at least makes sense to make a separate bmd when there's an animation involved.

@larsiusprime I just bare minimum tested on cpp and flash, and it's the same.

fs.pixels = bmd;
Log.trace(fs.pixels == bmd); // true, duh
Log.trace(fs.framePixels == bmd); // false, duplicated

Extending FlxSprite means we have a relatively useless animations property, among others, but then again a lot of core code is in there...

Edit: and disposing of either pixels or framePixels causes Error 2015 Invalid Bitmapdata. It makes sense to figure this out this year.

[larsiusprime]

I mean, perhaps we could add a "static" or "noAnimation" property to FlxSprite that would switch it into a single-bmp mode?

[MSGhero]

It'd be awkward to set that property at any time except for before loading in graphics, but maybe. The main thing would be framePixels = frame.parent.graphic (or something like that), but other things could be optimized out too.

[larsiusprime]

Well I mean you could make it part of the constructor or something.

[MSGhero]

Yeah cool, that did the trick. Saved 36 MB and my game doesn't have that bitmap tearing kind of lag.

I'd prefer a more complete pull req, but all I did was:

private var _noAnimations:Bool;
...
public function new(..., ?NoAnimations:Bool = false) {
    ....
    _noAnimations = NoAnimations;
}
...
public function getFlxFrameBitmapData() {
    ...
    framePixels = _noAnimations ? _frame.parent.bitmap : _frame.paint(framePixels, _flashPointZero, false, true);
    ...
}

[larsiusprime]

Neat!

[MSGhero]

Color transforms require a copy, rotations require a resized copy, flipping is slow unless you use a copy, and animations/atlases probably require a copy. So...you can really only move the sprite around.

Maybe the variable should be NoFancyStuff.

[larsiusprime]

heh :) Yeah it's a super gimped FlxSprite, but for a background all you need it to do is sit there or very rarely be moved somewhere else.

[iam-jim]

Normally a sprite should know nothing about animation. Because it's just a sprite. But that is flixel's way of doing stuffs.

[MSGhero]

What do you think, a parameter in FlxSprite or a new class in addons that extends FlxSprite and overrides the default behaviors?

[Beeblerox]

we/you could add simple sprite class but it would work only for non-flipped, non-alpha, no-tinting cases. I was thinking about separating current sprite into two parts: one for object representation (x, y, etc.) and the other for visuals. and FlxSprite should be subclass of it having all the functionality. This way could give us more flexibility. But in the past year my life significantly changed so i don't have time for my favorite hobby, which very sad to me. But hey, i got two weeks of vacation, which started today, so i'll try to do something with flixel, can't promise anythingm since i have other things to do (which were the cause i took the vacation), but let's see what will be done in these two weeks.

[Tiago-Ling]

My two cents / humble opinion: Visuals could be separated the same way that was done with animation. In the old days of HaxeFlixel (and Flixel), we had all animation logic inside FlxSprite, but later it was put into its own class which became the field animation of FlxSprite.

FlxSprite could do the same with its graphics, putting it inside its own class. The same way it contains animation it would contain an image (instead of being an image).

[Beeblerox]

@Tiago-Ling this is what i'm thinking about!

[Tiago-Ling]

@Beeblerox Cool! I think this would make things a lot more flexible in the long run :)

[sruloart]

@MSGhero what

bitmap tearing kind of lag

are you talking about? sounds like a bug with the bitmapData copy?

@Tiago-Ling I think separating the visuals is too much work at this point, and @Beeblerox already tried making a basic sprite, which didn't work out...that unless he thinks he (with some help?) can make it, obviously :)

What I'm suggesting is to auto-optimize: if a FlxSprite doesn't actually change its bitmapData's copy (flip, alpha, tint...whatnot) then don't use a copy (framePixels = frame.parent.graphic for example).

[Tiago-Ling]

@sruloart Again imho, the thing is that all changes like this will be a lot of work. If we never make them and keep making quick workarounds (hacks?) instead the framework will eventually become very bloated and with increasingly harder to solve bugs.

If i remember correctly the old approach was to create an intermediary class between object and sprite, which would mess with the inheritance chain, break all stuff and would really be a lot of work to fix. This new suggestion is a lot easier to make and will cause much less disruption. Of course it will demand some work, maybe a lot, but not nearly as much as the old proposition.

[iam-jim]

+1 for @Tiago-Ling's suggestions, many game engines uses this way including luxe, separating graphics/animation from sprite.

[MSGhero]

@Sruloart when a flash app is using a ton of memory, the internal render() starts to lag, though it still update()s normally.

[Beeblerox]

@Tiago-Ling yes, you're right that was a mess, that's why i've dropped it. separating graphics from sprite should be a much better solution

[Tiago-Ling]

@Beeblerox Btw, i'm not criticizing your work - i also thought that an intermediary class was a good idea before, but also came to the conclusion that it wouldn't work because of the way Flixel is set up.

I'm also short on time currently, but let me know if you need with anything - i'll be more than happy to help! :)

[Beeblerox]

@Tiago-Ling I'm not taking it as a critics at all, that's just the fact :)

[larsiusprime]

I think factoring out the graphics information into an internal component is a good idea -- it's a smart use of composition, and if done cleverly might not even break the user-facing API too much.

[MSGhero]

It might be possible to make it non-breaking (for instance get_pixels() returns _image.pixels). But should the graphics be completely hidden from the user, or should there be an image property like animation?

[sruloart]

I think right now there's a really nice structure emerging: a bitmapData (FlxGraphic) is placed (copied if necessary) inside an object that manipulates it (FlxFrame) according to commands given by a game entity (FlxObject) that holds a set of such commands (FlxSprite/FlxTile).

That means that a FlxSprite is simply a FlxObject with a linked frame, that changes its bitmapData in accordance with changing the object's position/angle/scale/flip/offset. So, no more updateHitbox() (it's the other way around see), but also no more FlxSprite.pixels/alpha (that should be FlxSprite.frame.pixels/alpha). This FlxSprite should continue to support collisions that takes in consideration its linked frame, because that makes sense, but that's all the functionality it should have.

Another point here that a FlxSprite is representing only one frame in the game world. Animation is simply a way of replacing its content at a fixed time-interval, so it should be handled somewhere not in the FlxSprite proper (FlxG.animation.add(sprite, name, frames, etc), like how tweens should be handled, and how music/sound already are).

[MSGhero]

@Beeblerox if you get the branch started and maybe leave some notes and TODOs when your vacation ends, the rest of us can pull to that branch and finish up.

@sruloart regardless of what makes sense in the framework, sprite.animation.add() makes more sense for a random user than going into FlxG.

[larsiusprime]

I agree with MSGhero on this last point; refactoring FlxSprite can make sense but using FlxG to drive an individual sprite's animation (and/or other properties) is a lot less intuitive then an operation on the object itself. Other than that I do like the ideas @sruloart proposed.

[sruloart]

So, what about the other extreme: using FlxSprite also as a local FlxG for convenience, i.e. you can control not only animation, but also tweens (FlxSprite.tween) and sound (FlxSprite.sound). Under the hood it would use the proper frontend to actually do something, like the way loadGraphic() adds a bitmapData for example.

This does require a tweenFrontEnd though :)

The basic FlxSprite would be called FlxObjectFrame (extends a FlxObject by linking a frame and changing it based on the changes made to the FlxObject). FlxSprite would then extend that by adding the different FlxG like easy-access functionalities.

No breaking change in terms of user API, just a better structure. This would even allow the user to create his own easy to access functions when extending a FlxSprite, like Enemy.ai.attack or Player.control.moveLeft etc.

Hopefully this change would also involve removing FlxTile completely.

[Tiago-Ling]

FlxObject -> FlxObjectFrame -> FlxSprite would be the same as the failed attempt at making FlxImage (or any other basic sprite class). It would bring the exact same problems for the same reasons.

Not a big fan of embedding more stuff into FlxSprite (even if it is just convenience methods). Imho sound and music could work in the same way as FlxTween, not the other way around (removing them from FlxG also).

[iam-jim]

The thing that bothers me about FlxTween and FlxTimer is they are not added in state hence I can pause the state but I have to separately pause tween and timers which makes them unusable in pause menu or in other places simultaneously.

I see no point adding more stuffs in FlxSprite. It's a just sprite, we should treat it this way.

[MSGhero]

FlxObject.tweens would resolve #1458... But not sound, that feels like it should be in FlxG from a user POV. Plus, tweens and timers should be paused with the state, but sounds are sometimes not paused (music is not, sfx are).

[sruloart]

@Tiago-Ling it doesn't have to be a new class, FlxFrame would be what you call "the FlxSprite's graphic" (not a direct quote, I know). It's not that different from the basic FlxSprite concept: you just keep all the non-basic game entity stuff (like .colorTransform/.alpha/.stamp/.draw...) as reference, but move all of the actual functionality to the FlxFrame.

I think we need a good discussion about what should and shouldn't go into FlxG, it's kind of a mess :)

@solewalker @MSGhero I'm just thinking out loud here, https://github.com/HaxeFlixel/flixel/issues/1087 says @JoeCreates would somehow rescue us all from this plugins' hell...

[MSGhero]

@Beeblerox I know your vacation is over now, but is there something we could be doing here? I updated flixel, undoing my changes that I forgot about, and I don't want to redo them.