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

[BUG] - Opacity not affecting shape() #334

Closed
ChasarooniZ opened this issue Oct 11, 2024 · 3 comments · Fixed by #336
Closed

[BUG] - Opacity not affecting shape() #334

ChasarooniZ opened this issue Oct 11, 2024 · 3 comments · Fixed by #336

Comments

@ChasarooniZ
Copy link

shape() not affected by opacity or fadeIn()

@Haxxer
Copy link
Collaborator

Haxxer commented Oct 12, 2024

@Codas we've shuffled the hierarchy around a bit on the canvas effect class, is this fixed in #275 ?

@Codas
Copy link
Contributor

Codas commented Oct 12, 2024

I haven't done anything to shapes.
in fact I totally forgot about them existing.
So probably not and I should make sure that shapes do still work.

Do you have an example for a sequence using shapes? @Haxxer or @ChasarooniZ

@Codas
Copy link
Contributor

Codas commented Oct 12, 2024

Okay I've looked into this a bit, and its a really unfortunate combination..

Previously (some time ago), an AlphaFilter was used to manage the transaprency of the effect being played.
This has been replaced with just using the alpha value of the sprite holding the effect itself, which really allowed practically all of the optimizations to work in the first place.

However something I did not consider was that for shapes, all filters of ther effect are also copied to the shape, which included the alpha filter. And the ColorMatrix filter I suppose, which I'll also have to fix I suppose. Fixing the latter is quite easy I guess... The filter still exists after all, just not in the filters array.

For the Alpha Filter, the sitaution is bit different.
The hierarchy of PIXI elements in a sequencer effect are like this:
CanvasEffect > <some containers for rotation, isometrics etc> > SpriteContainer

SpriteContainer itself holds tihs:

SpriteContainer
├─ Shape
├─ Sprite
│  ├─ Text

The SpriteContainer itself holds the Sprite playing the effect and potentially also the Shapes. Texts are actually added to the Sprite Itself.
The alpha filter simply does not exist anymore. The alphaFilter property for animations only exists for backwards compatibility right now and still modifies the alpha values directly.
This means that... I only see a few unfortunate ways forward here:

  1. Re-Introduce alpha filter.. I think this is rather unfortunate but has 99.9% backwards compatibility and is a more or less trivial code change.
  2. Add some edge-case handling for shapes and make sure that whenever a alpha value is set, we also set the value to any shapes. This means animations, loops and some other instances I think. This should also be like 99% backwards compatible with all animations, but introduces the most additional code I think.
  3. Move the shape inside the Sprite itself like texts. This is also a trivial change in code, but only like 95% backwards compatible? This would mean that things like spriteRotation suddenly also affect shapes. Maybe also other things like scaling? I'm not sure how often shapes and effects are used together thogu and if this really matters?
  4. Keep everything as right but simply set the alpha value on a container further up in the chain or even the CanvasEffect itself. This would maybe be the least backwards compatible?

I'm currently torn between 2 or 3... Maybe you have some other Ideas Haxxer?

And btw, since you asked about #275: Shapes are indeed broken on that branch right now. I'll try and fix it as soon as possible.

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

Successfully merging a pull request may close this issue.

3 participants