Am 26.01.2011 15:01, schrieb Andrea Canciani:
I think, You mean 0005, isn't it? Here the reason for the changes
a) SplashOutputDev::beginTransparencyGroup
The wrong setup of tx and ty caused segmentation faults in
Splash::blitTransparent()
This should not be a problem anymore with backward rasterization
You're not true: This is a basic bug in SplashOutputDev and it does't
depend on radial shading, so it should be corrected anyway.
b) Splash::pipeRun
If getColor return gFalse, we need to increment ALL pointers, use now the
already existing method pipeIncX
Cleanup is good, I agree.
c) Splash::shadedFill
shadedFill is now used by axial and radial shading, axial supports
antialiasing, radial not. Therefore we need to support either case in it.
Where does this difference come from?
Axial and radial shadings work the same way (the formula for the parameter
is different, but that doesn't affect the ability to antialias the
gradient, because
in splash antialiasing is performed by supersampling)
This is history, Gfx turns off vector antialiasing in every shading, the
only device which supports turning off antialiasing, is SplashOutputDev.
And probably it was introduced because of the implementation of
antialiasing in Splash. It was already so in xpdf, the ancestor of
poppler. When I implemented colorizing text in pattern colorspace I run
into this problem: when a font with a quite small font weight should be
filled with a pattern colorspace, I need vector antialiasing, otherwise
it looks awful. That was one of the reasons why I implemented axial
shading in Splash, where I switched it on again, s.
https://bugs.freedesktop.org/show_bug.cgi?id=30436
When I did that in first quarter last year, I also planned to implement
radial shading in Splash, but I hadn't a good idea how to do that and
not really time enough. So I started that during my xmas vacation. And
of course first what I did was turning antialiasing on again. But this
run into a problem with the PDF Albert mailed today (BTW, we had also in
some cases problems with antialiasing and axial shading, therefore I
switched it off if that kind of shading has a bounding box). But here I
found no parameter in the shading where I can decide whether to let it
on or not (other than in axial shading), so I decided to switch if off
again in case of radial shading. You can find that mail in the archives,
sent on 28th of december.
AFAICT disabling antialias on radial shadings is just a workaround to some
cornercases Albert hit. The problem exists with axial shadings as well, so
we should fix it for both of them instead of working around it for radial and
leaving it broken for axial.
I agree. But the main idea of my patch was not to solve the antialiasing
problems but to solve the problems with radial shadings. So let us do it
step for step instead of rewriting poppler completly :-) . And for me,
as propably for the most people here, poppler is a private pleasure, so
I can only help from time to time, when my family agrees :-)
Thomas
Ok, you got me, i'll regtest your patch. Please talk with Tomas to get a
better understanding of why we need 0003 and what it does :-)
Your patch regresses at altona_visual_1v2a_x3.pdf [1]
Okay, that's obviously. This PDF was the reason, why I removed antialiasing
again in radial shading. Andrea hadn't known that, and therefore probably
missed it.
Yes, I didn't know about it.
This is not really a regression of the radial rasterization, it is a bug
in the existing code, already hit for axial shadings, which my patch
also triggers for radial shadings, because it shares the same code
path between them.
Thomas worked around it by using non-antialiasiased radial gradients,
which apparently extends the clipping region a little. This causes a
regression in my radial-gradent-* tests with Thomas patch (adjacent
shadings overlap a little, even if they should be disjoint).
The circles are not totally filled in their right border. I stopped having
a
look all at the other files in my regtest that change (so it might regress
somewhere else) but makes no point having a look at all the other files
since
i'll have to do it anyway once you fix this regression.
Now my question is if I should commit cleaned Thomas patch or not.
If You agree, give Andrea's patch a final chance (I had a lot of chances :-
) ). Unfortunately I'm not able to make this small patch pieces today, so
here just the correction at end of SplashOutputDev.cc:
code snippet start
GBool SplashOutputDev::univariateShadedFill(GfxState *state,
SplashUnivariatePattern *pattern, double tMin, double tMax) {
double xMin, yMin, xMax, yMax;
SplashPath *path;
GBool retVal = gFalse;
// get the clip region bbox
state->getUserClipBBox(&xMin,&yMin,&xMax,&yMax);
// fill the region
state->moveTo(xMin, yMin);
state->lineTo(xMax, yMin);
state->lineTo(xMax, yMax);
state->lineTo(xMin, yMax);
state->closePath();
path = convertPath(state, state->getPath());
retVal = (splash->shadedFill(path, pattern->getShading()->getHasBBox(),
pattern) == splashOk);
state->clearPath();
delete path;
return retVal;
}
GBool SplashOutputDev::axialShadedFill(GfxState *state, GfxAxialShading
*shading, double tMin, double tMax) {
GBool vaa = getVectorAntialias();
// restore vector antialias because we support it here
setVectorAntialias(gTrue);
SplashAxialPattern *pattern = new SplashAxialPattern(colorMode, state,
shading);
GBool retVal = univariateShadedFill(state, pattern, tMin, tMax);
setVectorAntialias(vaa);
delete pattern;
return retVal;
}
GBool SplashOutputDev::radialShadedFill(GfxState *state, GfxRadialShading
*shading, double tMin, double tMax) {
SplashRadialPattern *pattern = new SplashRadialPattern(colorMode, state,
shading);
GBool retVal = univariateShadedFill(state, pattern, tMin, tMax);
delete pattern;
return retVal;
}
<<< code snippet end.
The patch above restores aliasing/antialiasing behavior (i.e. works
around the existing clip bug for altona, breaks radial-gradient-*).
I managed to reproduce the same problem with axial shadings in master,
so it's a problem in the existing code, rather than in radial patches.
This patch solves it for me on the altona test and preserves antialiased
shadings, which I would prefer it over disabling the antialiasing:
diff --git a/poppler/SplashOutputDev.cc b/poppler/SplashOutputDev.cc
index 0669f98..2ab8f6d 100644
--- a/poppler/SplashOutputDev.cc
+++ b/poppler/SplashOutputDev.cc
@@ -3495,7 +3495,35 @@ GBool
SplashOutputDev::univariateShadedFill(GfxState *state,
SplashUnivariatePat
// restore vector antialias because we support it here
setVectorAntialias(gTrue);
// get the clip region bbox
- state->getUserClipBBox(&xMin,&yMin,&xMax,&yMax);
+ state->getClipBBox(&xMin,&yMin,&xMax,&yMax);
+
+ xMin = floor (xMin);
+ yMin = floor (yMin);
+ xMax = ceil (xMax);
+ yMax = ceil (yMax);
+
+ {
+ Matrix ctm, ictm;
+ double x[4], y[4];
+ int i;
+
+ state->getCTM(&ctm);
+ ctm.invertTo(&ictm);
+
+ ictm.transform(xMin, yMin,&x[0],&y[0]);
+ ictm.transform(xMax, yMin,&x[1],&y[1]);
+ ictm.transform(xMin, yMax,&x[2],&y[2]);
+ ictm.transform(xMax, yMax,&x[3],&y[3]);
+
+ xMin = xMax = x[0];
+ yMin = yMax = y[0];
+ for (i = 1; i< 4; i++) {
+ xMin = std::min<double>(xMin, x[i]);
+ yMin = std::min<double>(yMin, y[i]);
+ xMax = std::max<double>(xMax, x[i]);
+ yMax = std::max<double>(yMax, y[i]);
+ }
+ }
// fill the region
state->moveTo(xMin, yMin);
To me it looks like a hack. The right thing to do would be to fix the
clip computation and/or the antialias code. This would fix both radial
and axial shadings (which currently are probably broken as well,
even though nobody noticed).
I'm not sure if my patch is an hack as well or if it is the right thing to do.
It changes the clip computation by ensuring that it is pixel-aligned, but
there is probably some problem in the splash antialiasing/clipping code.
This patch regresses my radial-gradient just like the patch proposed by
Thomas (introducing again the overlap between adjacent tiles), but
preserves antialiasing for all shading types.
Andrea
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
.
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
