Summary: | render artifacts in radial gradients | ||
---|---|---|---|
Product: | Ghostscript | Reporter: | Ian Bruce <ian_bruce> |
Component: | Graphics Library | Assignee: | Ray Johnston <ray.johnston> |
Status: | RESOLVED WONTFIX | ||
Severity: | enhancement | CC: | sphinx.pinastri |
Priority: | P4 | ||
Version: | master | ||
Hardware: | All | ||
OS: | All | ||
Customer: | Word Size: | --- | |
Attachments: |
radial gradient test case
Ghostscript render of radial gradient to PNG edge filter applied to Ghostscript output output to PNG of proposed new radial gradient algorithm edge filter applied to new algorithm output proposed new algorithm for radial gradients 6000x6000 Ghostscript render edge-filter 6000x6000 Ghostscript render 6001x6001 new algorithm render edge-filter 6001x6001 new algorithm render |
Description
Ian Bruce
2015-07-16 03:03:13 UTC
Created attachment 11799 [details]
Ghostscript render of radial gradient to PNG
Created attachment 11800 [details]
edge filter applied to Ghostscript output
Created attachment 11801 [details]
output to PNG of proposed new radial gradient algorithm
Created attachment 11802 [details]
edge filter applied to new algorithm output
Created attachment 11803 [details]
proposed new algorithm for radial gradients
Its not my area, but I'll make some general observations... If the artefacts are not visible to the eye, then I doubt we're going to want to change our current implementation, unless there's a very good reason. Improved performance would be a good reason, but you haven't suggested that this would be faster, indeed you say its 'not much worse' which isn't promising. Have you tried a comparison at a more realistic resolution, such as 600 dpi ? Remember, unlike MuPDF, Ghostscript's primary focus is on printing. The 'new algorithm' output does produce fewer radial steps in the blend, whcih is very nice, but then the Ghostscript output is at 144 dpi, which is rather coarse, so perhaps the stepping is to be expected. I suspect that Ghostscript deliberately degenerates shadings into trapezoids (or triangles) for a number of reasons (Ray might like to comment on this more authoritatively). One reason is performance, calculating and touching each pixel in a gradient is slow, especially as the resolution increases, so visually acceptable approximations are vital. Shadings are slow enough as it is. Also decomposing into trapezoids or triangles is the bottom level implementation in the graphics library, we don't generally deal with pixels (obviously the trapezoid or triangle rendering does). Hardware rendering implementations usually prefer to render triangles for instance. A patch in terms of libpng really isn't helpful, I don't know what half this code is doing, and there are NO comments whatever, so it would take me quite some time to find out what is actually going on if I were tasked with implementing it. I think this is really Ray's area, I imagine he'd like to comment as well. NB I've moved this from 'normal' to 'enhancement' as this is a proposed enhancement and not a bug. Created attachment 11806 [details]
6000x6000 Ghostscript render
Created attachment 11807 [details]
edge-filter 6000x6000 Ghostscript render
Created attachment 11808 [details]
6001x6001 new algorithm render
Created attachment 11809 [details]
edge-filter 6001x6001 new algorithm render
> If the artefacts are not visible to the eye, then I doubt we're going > to want to change our current implementation, unless there's a very > good reason. They are visible; the output of my algorithm is noticeably smoother. The edge-filtering just makes it obvious WHY. > Improved performance would be a good reason, but you haven't suggested > that this would be faster, indeed you say its 'not much worse' which > isn't promising. It seems to be about 30% slower, independent of resolution. This is just an initial version; there are probably things that could be done to speed it up. > Have you tried a comparison at a more realistic resolution, such as > 600 dpi ? Remember, unlike MuPDF, Ghostscript's primary focus is on > printing. See new attachments. As before, the "gs" files are the product of Ghostscript, and the "sq" files are from my program. > The 'new algorithm' output does produce fewer radial steps in the > blend, whcih is very nice, but then the Ghostscript output is at 144 > dpi, which is rather coarse, so perhaps the stepping is to be > expected. The 600dpi version (attached) shows exactly the same artifacts. In any case, what's relevant is the image resolution in dots, not dots-per-inch. > I suspect that Ghostscript deliberately degenerates shadings into > trapezoids (or triangles) for a number of reasons (Ray might like to > comment on this more authoritatively). The MuPDF bug said "the ideal approach would be to render radial shadings directly without tesselating them into triangles", which suggests that the issue is more one of implementation, rather than deliberate design. > One reason is performance, calculating and touching each pixel in a > gradient is slow, especially as the resolution increases, so visually > acceptable approximations are vital. Shadings are slow enough as it > is. Touching each pixel seems unavoidable, if it's going to have any shade at all. I kept the per-pixel computation to a minimum -- a few integer additions and a table lookup. > Also decomposing into trapezoids or triangles is the bottom level > implementation in the graphics library, we don't generally deal with > pixels (obviously the trapezoid or triangle rendering does). Hardware > rendering implementations usually prefer to render triangles for > instance. Decomposition into edge-matching triangle and quadrilateral patches is a significant calculation in itself, which is probably why my initial implementation comes within 30% of the existing one. As far as hardware rendering is concerned, it's probably worth considering texture maps. Where that's not available, other methods are required. > A patch in terms of libpng really isn't helpful, It's helpful in that it runs and is available for testing NOW. > I don't know what half this code is doing, and there are NO comments > whatever, so it would take me quite some time to find out what is > actually going on if I were tasked with implementing it. The entire algorithm is contained in the three functions mkgradient(), getpx(), and circle(), for a total of 36 (nonblank) lines of code. The only libPNG dependency therein is set_pixel(), which is surely self-explanatory. The significant features of the algorithm were explained in my first post; two nested for-loops along the x- and y-axes presumably don't require comment. Whatever other explanation is needed, I'm happy to provide. > NB I've moved this from 'normal' to 'enhancement' as this is a > proposed enhancement and not a bug. The same problem in MuPDF qualified as a "bug". Why is this different? As Ken mentions, indeed, the choice to tesselate the shading into linear color trapezoids and triangles is intentional and some work has been done to make the choice of vertices optimal to minimize 'gap' effects and double painting of pixels. The use of linear color traps/triangles reduces the size of the 'clist' display list and can be used with hardware Gouraud shaders (some of our high end customers have ASIC's to accelerate basic rendering). Also Ghostscript implements the smoothness control of PS and PDF (SM in a PDF ExtGState dictionary). This allows users to trade off the quality vs speed of shading (gradient) fills. (In reply to Ian Bruce from comment #11) > > If the artefacts are not visible to the eye, then I doubt we're going > > to want to change our current implementation, unless there's a very > > good reason. > > They are visible; the output of my algorithm is noticeably smoother. I don't disagree, I'm just pointing out that 'good enough, and fast' often trumps' better but slow'. > > Improved performance would be a good reason, but you haven't suggested > > that this would be faster, indeed you say its 'not much worse' which > > isn't promising. > > It seems to be about 30% slower, independent of resolution. 30% is a lot, shadings are a significant overhead already (though possibly not specifically radial shadings). > > Have you tried a comparison at a more realistic resolution, such as > > 600 dpi ? Remember, unlike MuPDF, Ghostscript's primary focus is on > > printing. > > See new attachments. As before, the "gs" files are the product of > Ghostscript, and the "sq" files are from my program. I was really more interested in a performance comparison at the higher resolution, which you gave above. > The MuPDF bug said "the ideal approach would be to render radial > shadings directly without tesselating them into triangles", which > suggests that the issue is more one of implementation, rather than > deliberate design. For MuPDF possibly this is true, but this is a Ghostscript report. > > A patch in terms of libpng really isn't helpful, > > It's helpful in that it runs and is available for testing NOW. Well it doesn't help *me* because I don't understand it. > The entire algorithm is contained in the three functions mkgradient(), > getpx(), and circle(), for a total of 36 (nonblank) lines of code. With no comments..... > The significant features of the algorithm were explained in my first > post; two nested for-loops along the x- and y-axes presumably don't > require comment. Whatever other explanation is needed, I'm happy to > provide. Its been years since I wrote any C++, and I can't make head or tail of this. Possibly if it was commented I could. On the other hand, its not likely ot be me who does anything with it..... > > NB I've moved this from 'normal' to 'enhancement' as this is a > > proposed enhancement and not a bug. > > The same problem in MuPDF qualified as a "bug". Why is this different? Its clearly an enhancement, perhaps the MuPDF developers don't care about differentiating, I like to keep things tidy in the Ghostscript world. Unfortunately, the "new algorithm" cannot be used directly in Ghostscript, but it requires a lot of extra development, which would far exceed the complexity of the submitted code. For instance, Ghostscript has the current transformation matrix, banding, and clipping. The submitted code doesn't have anything like this. If Ghostscript will ever add radial gradients to the set of supported primitive operations, coding something similar won't be too difficult. We value every contribution and permanently keep every bug report, but we would also like to reduce the number of open bug reports to focus on more urgent issues. |