elm-community / elm-webgl

Moved to elm-explorations/webgl
http://package.elm-lang.org/packages/elm-explorations/webgl/latest
BSD 3-Clause "New" or "Revised" License
95 stars 22 forks source link

Add support to disable premultipliedAlpha #19

Closed w0rm closed 7 years ago

w0rm commented 8 years ago

It is not possible to disable premultipliedAlpha which is very useful to have transparent sprites.

Currently the transparency issue can only be solved by hacking the shader code.

Zinggi commented 8 years ago

This is quite a bummer. I'm currently working on this little 2d rendering engine and this would be very much needed for any 2d game with sprites.

I've read the contributing.md, but this can't be added as a PATCH.

Best would be to add something like

toHtmlWithEvenMore
    :  { alpha: Bool, depth: Bool, stencil: Bool, antialias: Bool, premultipliedAlpha: Bool  }
    -> List FunctionCall
    -> List (Attribute msg)
    -> List Renderable
    -> Html msg

(Info to these properties here)

This would be a MINOR change.

I left out two properties because I don't really understand the first one and because I think the second one would be useless in elm:

I'm currently also doing the little shader trick from @w0rm combined with BlendFunc (One, OneMinusSrcAlpha), but this isn't a good solution. The problem with this solution is that you also have to change the rendering order. This makes rendering multiple objects very unintuitive for a user.

nphollon commented 8 years ago

Possible alternative to @Zinggi API suggestion: What if we extended the Capability type to cover these attributes?

Then there would be no need to add a new toHtml-type function... you could just call Disable PremultipliedAlpha to your FunctionCall list. This would be a less invasive API change.

I realize that premultipliedAlpha is not actually a function call, but from an implementation perspective I don't see a problem with treating it like one. What would y'all think? Would this be confusing?

Zinggi commented 8 years ago

@nphollon, I like the suggestion, but there is a problem with it.

You can use List FunctionCall as an agrument to renderWithConfig too. Now with your suggestion you could then write a nonsense renderWithConfig [Enable PremultipliedAlpha] which doesn't make sense on a per render basis, as it (premultipliedAlpha) only applies to the top-level canvas element.

It would be a way to avoid bumping the version, but it wouldn't be the best API.

I think someone has to bother Evan for this. It's not critical, but it would be a big improvement. I would go ahead and implement it, but first I have to know whether a pull request would even be accepted.

nphollon commented 8 years ago

Good point, I had forgotten about renderWithConfig. (It would have required a version bump anyway, since it would involve changing a type definition.)

As for whether the PR would be accepted, you can get approval from either @evancz or @johnpmayer. We also have #32 waiting for approval at the moment. I think it would make sense to release the updates together if they both get the thumbs up.

Zinggi commented 8 years ago

All right, I'll try to get my hands dirty on this one. Will take some time though, as I haven't done native stuff yet.

Zinggi commented 8 years ago

Ok, so I implemented this here: https://github.com/elm-community/elm-webgl/pull/38

w0rm commented 7 years ago

This is implemented as the part of 2.0.