GiovineItalia / Gadfly.jl

Crafty statistical graphics for Julia.
http://gadflyjl.org/stable/
Other
1.91k stars 250 forks source link

`FillPrimitive` is not being correctly rendered when using PGF backend #1290

Open lassepe opened 5 years ago

lassepe commented 5 years ago

When using PGF export, I fail to produce working code for plots using Geom.violin. Here is a minimum working example. Exporting to PDF via Cairo works, but the PGF export does not seem to work. Am I doing something wrong with compiling the PGF using latexmk, or can someone reproduce this behavior?

using Gadfly
using Random
using Cairo # for PDF
using Fontconfig # for PDF

data = vcat(randn(100) .+ 10, randn(100) .- 10);
p = plot(y=data, Geom.violin);

# Works as expected:
draw(PDF("out/test_violin.pdf", 10cm, 10cm), p)
run(`xdg-open out/test_violin.pdf`)

# Does not work:
draw(PGF("out/test_violin_pgf.tex", 10cm, 10cm), p)
cd("./out")
run(`latexmk -pdflatex=lualatex -pdf test_violin_pgf.tex`)
run(`xdg-open test_violin_pgf.pdf`)
cd("..")

Expected output: image

Actual output: image

bjarthur commented 5 years ago

dang. we do test Geom.violin with PGF, but our regression testing doesn't make sure it actually displays properly. do any of the testscripts work for you in PGF? that's the one output format i don't use.

lassepe commented 5 years ago

If I run the Gadfly tests on my machine all tests succeed. However, when trying to compile the tex files created by PGF, there are some that fail to compile because they contain invalid latex code. These are:

issue177.tex
issue961.tex
nan_skipping.tex
title.tex

For all other files I get some kind of compiled PGF. I attached an archive of all .tex files generated by the tests and the .pdf files compiled from these. Also, I added the corresponding .pngfiles generated by the tests as a "ground truth" since I assume that these will be well tested.

I don't have time right now to go through all of them but I can see that the violin polts were not generated correctly:

PGF: image

PNG: violin

The same problem seems to occur for other plots. Here is the what happens for the colored_ribbon test script:

PGF: image

PNG: colored_ribbon

Appendix

Archive of test output: pgf_tests.tar.gz

bjarthur commented 5 years ago

the common problem between violin and colored_ribbon is the FillPrimitive is not being correctly rendered. compare the code for PGF to PDF.

lassepe commented 5 years ago

Okay, I see. So I guess it makes sense to move this issue to the Compose.jl project?

bjarthur commented 5 years ago

no, leave it here. the developers are all the same.

Mattriks commented 5 years ago

Actually the problem is in the draw function for PGF polygon. This line should be

n<=1 && return

Editing this line might also help:

property.color.alpha < 1.0 && (img.fill_opacity = property.color.alpha)

These 2 edits might be sufficient for simple charts, but neither of these edits will solve bigger issues, which will affect nested contexts (scopes in PGF).

bjarthur commented 5 years ago

@Mattriks unless you or someone else has short term plans to fix the bigger issues, perhaps it's worth the minimal effort to submit a PR to incorporate the two small changes you outline above?

lassepe commented 5 years ago

Editing this line might also help:

img.fill_opacity < 1.0 && (img.fill_opacity = property.color.alpha)

Is this really the check that you intended here? Or should it be property.color.alpha < 1.0? Also, maybe the right thing to do is a clamp(img.fill_opacity, 0.0, 1.0).

Mattriks commented 5 years ago

Yes that's probably what I intended (I edited my post above). The issue is that img.fill_opacity should inherit from the parent context, but in Compose all "string" colors e.g. fill("blue") have alpha = 1.0.

gr1 = compose(context(), fillopacity(0.3),
    (context(), Compose.circle(0.5, 0.5, 0.2), fill("blue"))
)
draw(PGF("./test/gr1.tex", texfonts=true), gr1)
run(Cmd(["latexmk", "-outdir=./test", "-pdf", "./test/gr1.tex"]) )
lassepe commented 5 years ago

Good to know. I think I have seen this showing up as a bug before (things with a string color ignoring the fill property).

lassepe commented 5 years ago

However, this sounds like it introduces weird behavior, if the user want's the child to be explicitly non-transparent. E.g. if the parent has fillopacity(0.3), the user should still be allowed to give one of it's children "fillopacity("1.0")" which will be broken with the suggested fix.

Mattriks commented 5 years ago

fillopacity is a seperate property:

# This should override the parent fill opacity in this context:
(context(), Compose.circle(0.5, 0.5, 0.2), fill("blue"), fillopacity(1.0))

The line I'm editing above is for the fill property, which may be fill("blue") or fill(RGBA(0,0,1,0.3)), and hence changes the fill opacity separately from fillopacity().

lassepe commented 5 years ago

Okay. But the alpha==1 in fill(RGBA(...)) will always be ignored and overwritten by the parent property, right? But I guess that is okay (seems to be the case everywhere else in Compose as well)

Mattriks commented 5 years ago

Yes a=1 will be ignored in e.g. RGBA(r,g,b,a), because of the fill(string) issue mentioned above. My opinion is that fill("blue") and fillopacity(0.3) is more user-friendly than RGBA-style syntax, and hence the former should be favored in a user-friendly graphics package. Also RGBA aficionados can still do RGBA(r,g,b,0.99).