SpriteManager WebGL transparency draw issues and improves

Babylon’s SpriteManager draws sprites in wrong way: it first calcs depth, and than draws all pixels. It produces result like this:


Top sprite was added before white one and whole alpha was overlapped.

According to this solution. We have to draw opaque and transparent pixels in sequence. Here’s working code. Shader uses negative alphaTest values to cut of opaque pixels.

  • First feature request: adjust draw algorithm.
  • Second feature request: my solution only fixes draw into one draw call (or update() of one SpriteManager). For work it in all draws we need to call all draw operations for opaque pixels first, then for transparent. It requires changes in SpriteManagerComponent or something, don’t sure.
  • Third feature request: remove all cycles in render() loop. It’s possible to draw thousands of sprites without any performance issues.

    Don’t sure i’m in right to share my code, but solution is simple - instead of storing all values in Sprite object and copy it into array, it stores values in vertex buffer directly, so you need only to bindBuffer. Sprite values link example

I also posted detailed info about my research. This post on russian, but has some detailed pictures

So let s try to summarize here :slight_smile:

  1. sounds like a bug and it would be awesome if you could share a repro in the playground for us to look into instead of the picture ?

  2. we are not making distinction at this time on sprite order so yes this would require more rework.

  3. Is it not equivalent to lose a lot of features ?

Oh… Does SpriteManager supports only billboard mode? Another feature request:) (I actually doesn’t need non-billboard mode in engine already, but it is valuable feature)

  1. It isn’t babylon bug, more opengl fundamental problem. Usually you have to draw opaque geometry first and than transparent. But with sprites it draws in almost random order. Here’s sandbox.
  2. Here’s demo on case somebody will fix first problem :slight_smile: It looks same for now, but uses separate draws operations for each “layer” and it harder to solve
  3. Sorry i added wrong link for this case. Here’s correct one.
    Only animations may suffer, but anyway it better to process in separate loop. (As separated instances)
    It’s the way to boost performance of scene into the sky. CPU loops is bottleneck on any device, there’s no need to this._appendSpriteVertex(offset++, sprite, 0, 0, baseSize); each frame

Considering all your requirements I wonder if you should not use instances or SPS (@jerome) to speed up normal plane renderings ?

Sprites are indeed defaulting to billboard.

As a workaround, you could manage yourself the sprites that are opaque and transparent, by creating two separate sprite managers, one for opaque sprites and the other for transparent sprites, as you did in the PG: https://www.babylonjs-playground.com/#BV01S5#1

To simulate what is done by Babylonjs for regular meshes (opaque meshes are displayed before transparent meshes and transparent meshes are displayed with disableDepthWrite = true), you should create the opaque sprite manager first, to ensure opaque sprites will be draw first. Also, you should set the blendMode of the opaque sprite manager to none, so that the depth buffer is written for those sprites:

https://www.babylonjs-playground.com/#BV01S5#2

See the comments I have added in the PG for more explanations.

2 Likes

Thanks for solutions, but i use rewritten SpriteManager. At least I can’t afford useless CPU loops in first place. Why we even have to argue about calling this monster each frame for each sprite?
In my first post i attached working render code i use in my SM. I started this topic more about improving babylon.

@Evgeni_Popov thanks again but each SM has 2 draw calls already. It doesn’t good to twice it up. (Actually what you talking about more about second issue when you need 2 depth buffers for opaque end transparent draws).

offtopic-like:

@sebavan i think SpriteManager has to be wrapper around SPS to be “good solution” on engine level. With atlases, clicks and etc. support. Basically they do same job, and i don’t understand why codebase needs solution duplicates.

About billboarding. I got your point about “meaning” of sprites and 3d space usage, but here’s my case:
2020-03-06-214632_681x410_scrot
This is isometric game with usual prerendered 2d textures. We can draw same scene with billboard mode, but we need correct depth, so some of them flat, some vertical:

.
Soo yes, it just planes with textures on different angles, but doesn’t sprite already plane with texture with same work? I mean, when “textured plane” becomes sprite if someone uses it same way. Why can’t we just say “Sprites is textured planes and you can draw as billboard or whatever angle you want”

P.S. Didn’t mean to be rude or something (If i was). Sometimes i don’t sure how to be nice in this language

No worries totally understand your point :slight_smile:

The thing is our current solution fits with what we are doing at the moment as the main goal of the sprite is to manage spritesheet and billboard.

Here you are trying to extend the system which is awesome but I wonder how we could make it a feature keeping in mind our thrive for back compat.

Let s sync with @Deltakosh in a week once his vacations will be over.

1 Like

Note that your solution is specific to your needs, whereas Babylon has to be more generic.
Notably, the loop:

  • calls _animate for each sprite in case they are animated
  • rewrites the vertex data to take into account:
    • sprites added/removed to the sprite manager since last render
    • sprite isVisible property (possibly) modified since last render

If your sprites are not animated and you don’t modify the isVisible property of any sprite, then you can indeed remove the loop and generate the vertex data a single time once all sprites are added to the sprite manager. But understand that it is a specific case and that Babylon must be more general than that.

Maybe, a freeze() / unfreeze() method could be added to skip the loop. That would achieve what you want: the loop would be skipped and the rendering would be performed with the vertex data generated on the last render before freeze has been called.

Certainly because sprites and sprite managers existed long before SPS came in.

Did you see this?

Always thought that SpriteManager is relatively new feature according to how raw it looks.

My solution fits all needs that SpriteManager does and does not. That’s because i write about it here.

As i told before - yes, it only one thing you need to iterate on CPU if you need texture atlas frames. But you can iterate “animated” sprites in another loop. You probably doesn’t need it to iterate each frame according to you animation speed.

It can be done in moment of removing/adding/change anything OR by “dirty” flag/list. Why iterate over maybe million elements when only one maybe was changed. https://www.babylonjs-playground.com/#BJKS09#1 it’s dead on 100k already and we even hasn’t any logic in this scene

I defined optional BILLBOARD flag for myself so i can use direction as uniform to draw all sprites in one direction(camera) or direction as attribute to draw in in any directions. Anyway there’s big alpha and performance issues and billboarding is minor problem.

I like the idea of freeze/unfreeze :slight_smile: and the direction one. We ll define in a week how/if we integrate with @Deltakosh.

Indeed.

I guess the first implementation didn’t need more than that and nobody needed / asked for improvements until now.

I’m sure the core team would be all for a PR that would improve the sprite management in the way you describe without breaking backward compatibility, but I think we need to wait for @Deltakosh inputs about all this.

1 Like

For me freezing/unfreezing is just longer (in code lines) way to set dirty flag through setter

//Sprite class somewhere
setDirty(val){
   this._dirty = true;
   this._manager.addToDirtyList(this);
}

set width(val){
   this._width = val
   this.setDirty(true)
}

//===
//code somewhere
sprite.width = 100;

But it more about engine code policy and personal preferences.

And I also don’t think there’s will be better way to update values than direct link into vertex array

//Sprite class somewhere
set width(val){
   //writes val directly into 4 vertices
   this._manager.setVerticesValue(this.index, WIDTH_INDEX_SHIFT, val);
}

It saves twice memory (Actually 3x because default js float is 64 bit when vertices in Float32Array) and allows you just call draw operations without any additional updates

Do you want to start a PR to see how we can improve SM?

Remember that we do not break backward compatibility so it has to be compatible

You should really be cautious about your tone buddy…

So are we considering this a specific use case or are we proposing some changes? I can prolly fix a few things if I know what the exact issue is but as far as I can tell this is all very situational.

I think @Evgeni_Popov solution of using two managers to handle the opaque vs transparent meshes would be the solution.

Every time I see someone post Sprite anything I cringe now because in the back of my mind I’m like ohhhh noooo what did I mess up… >_<, but this sounds like stuff from before me.

We are totally not against a PR which does not break back compat and bring new features / perfs :slight_smile:

Didn’t mean to offend anybody personally. Just facts:

  • It has ordinary opengl blending issues which makes it unusable for production
  • It very slow and it makes it just more unusable for production
  • It hasn’t any debug options available for rest of engine like gizmos or wireframes. It doesn’t matter by itself but means that engine uses different render logic for similar entities

Probably i should offend anyone who protects code that obviously doesn’t ready for commercial usage. But it’s free framework and i’m good with any sources i can use if it saves my time anyhow.

As i told before there is two major issues - alpha and performance. Another things more about features and minor tweaks. First one is really common for 3d and web has tons on solutions with different outcomes. Second one… Do we really need to argue about how bad linear complexity in render loops? Just because i’m already doing this, here’s is my point: according to babylon’s golden rules we should to consider faster solution as better one if another things are equal. “Ultimately the goal is to render more with less resources.”

Here are two things requiring “proposing some changes”:

  • 3d alpha blending (semi-transparency)
  • and linear time complexity functions in render loop

For first thing i have partial solution. It works in same two draw calls as original SpriteManager and draws semi-transparency pixels as they have to be. But it can’t handle multiple SpriteManagers because they just draw on top of each other.
For second one i suggest at least update vertex arrays when something changes.
There’s lot of links and explanations i posted before if you interested in details.

I guess it’s totally ok to write code that has some bugs or someone doesn’t like. Even if solution has issues it’s still doing job it was created for, right?

khm… anyways

I probably should. But don’t sure i can provide full solution because it requires deeper understanding of framework pipelines.
Maybe it’s better to fix bugs in first place and than consider about improvements in another thread?
I also don’t see any unit test requirements in contributing doc. Does it ok to just throw in any changes without tests?

//—
Sorry for bothering you guys if i did and please forgive my language issues

1 Like

We have both unit and validation tests. For visual features it might be easier to just add new use cases there to prevent regressions.

I guess here the easiest is you start with a PR for the bug fix and we ll go from there to see what we miss ?

I can easily imagine that at the time this code was written, the need / spec was for a small number of sprites, maybe of some tens, as it is generally the case I would say. Needing hundred of thousand sprites as in your case is pretty uncommon. So, given these requirements, I think the current code fits the bill. Improving the code in this case would not show any gain in the big picture, when you take into account everything else done during a frame.

No you should not:

  • as you say below, it’s an open source project, so nobody owes you anything (no reason to offend anyone). You should shift a bit your mind about this
  • nobody is protecting this code, several core members told you they were ok for a PR to improve things

It’s not to say we can’t criticize things to try to improve them, as long as you (precisely) don’t offend / be rude with people.

Note: I’m not the one who wrote the code and I’m not from the core team, so I don’t feel “offended”, still I can easily see how whoever wrote the code would be quite pissed off by your careless comments…

Indeed, and because it is open source you can contribute to make it even better.

Peace! :slight_smile:

2 Likes