Switch back from using RGBA for fill color

[tlnagy]

Problem

As pointed out by https://discourse.julialang.org/t/gadfly-svgs-render-without-color/16804/, using RGBA with fill doesn't appear to be widely supported outside of browsers since it's not part of the SVG 1.1 spec.

This is also noted in the MDN SVG docs:

@bjarthur correctly noted that RGBA is permitted in the SVG2 spec (https://github.com/GiovineItalia/Compose.jl/issues/318#issuecomment-429625909), but that spec isn't widely adopted by major packages like Inkscape, MS Office, etc. I'm labeling this a bug since it's breaking and a high priority to fix.

Solutions

The problem is that just reverting #319 isn't sufficient as then #318 will resurface. We might need to revert #314 as well until we get this worked out. @Mattriks thoughts?

We need a good solution for https://github.com/GiovineItalia/Compose.jl/pull/314#issuecomment-429561491. The problem is that the vector fill-opacity is being printed without knowing whether a scalar fill-opacity is present so we end up with the double fill-opacity issue. Can the vector fill-opacity detect the presence of the other one and clobber it or multiply the opacities? The latter is preferable.

[Mattriks]

I think the quick solution is to revert #319, and implement my suggestions prior to bjarthur's rgba suggestion: see my initial https://github.com/GiovineItalia/Compose.jl/issues/318#issue-369859071. My suggestions were for 2 changes:

• In Compose: throw a warning if a user tries to use e.g. an RGBA color and fillopacity together.
• In Gadfly: probably option i: deprecate theme.panel_opacity

[Mattriks]

Should I start by submitting a PR to Gadfly to deprecate theme.panel_opacity?

[tlnagy]

In Compose: throw a warning if a user tries to use e.g. an RGBA color and fillopacity together.

Ideally, we should be able to handle situations like this via multiplication of the two opacity values together, but I think this is an acceptable compromise in the mean time.

Should I start by submitting a PR to Gadfly to deprecate theme.panel_opacity?

This seems reasonable because AFAICT it's the only place we use opacity separately (lowlight_opacity never worked and is deprecated anyway). We should probably advertise the use of RGBA colors for theme.panel_fill.

[Mattriks]

Also, panel_opacity never worked either (it had a default value of 1 - which would have rendered the plot panel black). When I fixed fillopacity in Compose, Gadfly panel_opacity started working, which had 2 side effects:

• The double printing of svg fill-opacity
• The panel turned black so I had to change the default panel_opacity to 0.

Time to deprecate it!

[Mattriks]

The next step here is to revert #319 - I assume you're going to do that sometime.

[tlnagy]

324 is the revert PR, where should I add the warning that you mentioned?

[Mattriks]

Here's a test function:

using Colors,  Compose
Property = Compose.Property
FillPrimitive = Compose.FillPrimitive
FillOpacityPrimitive = Compose.FillOpacityPrimitive

function svgalphatest(properties::Vector{Property})
    has_fill_opacity = any(isa.(properties, Property{FillOpacityPrimitive}))
    !has_fill_opacity && return
    is_fill = isa.(properties, Property{FillPrimitive})
    fillproperties = properties[is_fill][1]
    has_alpha = any([alpha(x.color) for x in fillproperties.primitives].<1.0) 
    has_alpha && @warn "For svg transparent colors, use either e.g. fill(RGBA(r,g,b,a)) or fillopacity(a), but not both."
end

function push_property_frame(img::SVG, properties::Vector{Property})
    isempty(properties) && return
    svgalphatest(properties)     
# rest of the function ...
end

And some examples. The first 3 examples don't trigger the warning:

img = SVG()
push_property_frame(img, [fill("blue"), fillopacity(0.3)])
push_property_frame(img, [fillopacity([0.5, 0.3]), fill(["blue","red"])])
 push_property_frame(img, Compose.Property[fill(RGBA(0,0,1,0.3))])
 push_property_frame(img, [fill(RGBA(0,0,1,0.3)), fillopacity(0.3)])
 push_property_frame(img, [ fillopacity([0.5, 0.3]), fill([HSVA(0,1,1,0.3),RGBA(0,0,1,1.0)])])

[Mattriks]

This can be incorporated - I assume you'll add it to #324?

[tlnagy]

That's the plan, I'll add it in a couple days if no one gets to it faster. I'm kinda busy right now.