I beg your pardon; my patch from yesterday needs as revision. Please use
the patch file attached in this mail; it is the correct one.
The description of my last mail is correct however (see below).
Thanks,
Christian
Am 02.07.2010 21:12, schrieb Christian Feuersaenger:
Dear Albert,
I have improved my shading Type 4/5 bugfix as suggested.
More precisely, I
- incorporated your patch file (which removed the debug things),
- improved the code comment which you found misleading (see your mail
below),
- wrote a clean interface to implement the runtime optimization which
reduces the new/delete usage.
The last point is probably the most interesting one: I suggest to use a
"ReusablePathIterator" in GfxState which allows to re-iterate over a
given (frozen) path as often as one wishes, see the code documentation
for details.
Attached you find
- the patch, relative to
poppler-0.14.0-3-gb2427d0
- an updated test.pdf (let me know if you are interested in these test
cases -- they will probably grow even more when I am finished with the
*real* gouraud shader).
With your permission, I will also try to provide bugfixes for the
Shading Type 6 implementations (see the test.pdf).
Best regards
Christian
Am 28.06.2010 18:08, schrieb Christian Feuersaenger:
Hi Albert,
thank you for investing your time in my contributions! I will look into
your suggestions and commit a new patch proposal once I found a more
satisfying solution.
I have now test pdfs (and ready applications) for pdf Shading Type 6 and
Shading Type 7 (patches). There are several graphics bugs which seem
like bogus memory initialisation when displaying them, so I'll look into
them as well (unless someone else is currently doing so).
Concerning the low level linear interpolation shader, I've come quite
far, yet it needs some further attention.
Best regards
Christian
Am 27.06.2010 18:55, schrieb Albert Astals Cid:
A Diumenge, 20 de juny de 2010, Albert Astals Cid va escriure:
A Divendres, 18 de juny de 2010, Albert Astals Cid va escriure:
A Dimecres, 16 de juny de 2010, Christian Feuersaenger va escriure:
Dear Poppler Developers,
attached you find my bugfix proposal to improve rendering of Type 4/5
Shadings (Gouraud Interpolated Triangle Shadings).
What it does is:
1. implement the Function lookup to fix buggy display of
parameterized
shadings,
2. Improve the triangle refinement control such that it runs 5 times
faster,
3. Share path memory between successive flat triangles which gains
another 10% runtime improvement (avoids many new/delete operations).
I believe the changes are stable and should work without problems.
The
attached patch file patches against branch poppler-0.14 . Note that I
changed the access policies to GfxSubPath: it has no setter methods
which allows to re-use the same path with different coordinates
(Point
3 of the list above).
I'm running the regression testing with your patch, if no
regressions are
detected then we can look at the code and see if we like it or not :D
Thanks for the patch :-)
Passed the regression testing fine, will have a look at the code later
today.
It took a bit more than expected, sorry about that.
I've removed the debug code and the #ifdef for the speedup, since
yes, we
obviously want the speedup, i'm attaching my work in progress patch.
I like you to do two things though.
Please change
+ // preallocate a path.
+ // Overwriting the coordinates saves about 10% at runtime for larger
+ // shadings.
+ state->moveTo(0., 0.);
+ state->lineTo(1., 0.);
+ state->lineTo(0., 1.);
+ state->closePath();
to a proper call like state->preallocatePath(); or something like
this, since
that four lines smell "big hack" and even though it is a hack i think
it makes
sense to have a function for it and not just create a fake triangle.
Can you please explain/rephrase
+ // this always produces output -- even for parameterized ranges.
+ // But it is wrong for parameterized colors:
Thanks for the patch and sorry again for the delay.
Albert
Albert
Albert
Looking forward to your opinions,
best regards
Christian
PS
I will send patch files for the implemented draft of *real* gouraud
interpolated shadings when they become more or less stable.
Am 14.06.2010 23:07, schrieb Albert Astals Cid:
A Dilluns, 14 de juny de 2010, Christian Feuersaenger va escriure:
Hi Albert,
thank you for the fast reply and your positive answer!
I will use the next days to apply the xpdf patches to libpoppler.
I intent to patch the patched triangle refinement to your stable
branch, which appears to be the poppler-0.14 (?). I consider it to
be a bugfix.
Right poppler 0.14 is our stable branch.
The interpolated shader is not yet stable and may need some
revisions. I will continue working on it, preferrable on a separate
branch. If you like, I can try to make a www git repository
somewhere such that you can fetch my changes and merge them to
whereever you want.
Personally i'd prefer that you send the patches to the mailing list,
makes easier for more people to see them and comment if they feel
like.
Albert
For the moment, you find my test.pdf attached. it has been
generated
with latex and the unstable version of \usepackage{pgfplots},
http://pgfplots.sourceforge.net/; I also attached the .tex sources.
So, thank you for the positive feedback.
Best regards
Christian
>> > I am sure my additions are valueable and propose them for
>> > usage in libpoppler: the bugfix/speed improvement is
>> > directly usable and
stable
>> > and the lowlevel shader will need some more time. I could
>> > also
use some
>> > advice to get the integration with transparency, blending
>> > and
whatever
>> > correctly. I started with the xpdf sources, so I would
>> > also
appreciate
>> > any hints how you communicate changes between xpdf and
>> > libpoppler
if you
>> > are interested in my proposal.
>
> Yes, we are interested in your patches, basically what we would
> need
you is to
> provide a patch over poppler sources, you can choose wheter you
> want
to make
> it against git master branch, git poppler-0.14 branch or the
> released
0.14.0
> tarball.
>
> Also it would be interesting to have the PDF you have used for
> testing.
>
> Thanks,
>
> Albert
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
diff --git a/.gitignore b/.gitignore
index d000352..be47f98 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,3 +25,4 @@ poppler.pc
stamp-h1
gtk-doc.make
*.o
+.*.sw?
diff --git a/poppler/Gfx.cc b/poppler/Gfx.cc
index 221caef..cb6379c 100644
--- a/poppler/Gfx.cc
+++ b/poppler/Gfx.cc
@@ -99,11 +99,19 @@
#define radialColorDelta (dblToCol(1 / 256.0))
// Max recursive depth for a Gouraud triangle shading fill.
+//
+// Triangles will be split at most gouraudMaxDepth times (each time into 4
+// smaller ones). That makes pow(4,gouraudMaxDepth) many triangles for
+// every triangle.
#define gouraudMaxDepth 6
// Max delta allowed in any color component for a Gouraud triangle
// shading fill.
-#define gouraudColorDelta (dblToCol(1 / 256.0))
+#define gouraudColorDelta (dblToCol(3. / 256.0))
+
+// Gouraud triangle: if the three color parameters differ by at more than this percend of
+// the total color parameter range, the triangle will be refined
+#define gouraudParameterizedColorDelta 5e-3
// Max recursive depth for a patch mesh shading fill.
#define patchMaxDepth 6
@@ -112,6 +120,8 @@
// fill.
#define patchColorDelta (dblToCol(1 / 256.0))
+#define GOURAUD_SAVE_ALLOC
+
//------------------------------------------------------------------------
// Operator table
//------------------------------------------------------------------------
@@ -3087,22 +3097,63 @@ void Gfx::doRadialShFill(GfxRadialShading *shading) {
void Gfx::doGouraudTriangleShFill(GfxGouraudTriangleShading *shading) {
double x0, y0, x1, y1, x2, y2;
- GfxColor color0, color1, color2;
int i;
- for (i = 0; i < shading->getNTriangles(); ++i) {
- shading->getTriangle(i, &x0, &y0, &color0,
- &x1, &y1, &color1,
- &x2, &y2, &color2);
- gouraudFillTriangle(x0, y0, &color0, x1, y1, &color1, x2, y2, &color2,
- shading->getColorSpace()->getNComps(), 0);
- }
+ // preallocate a path.
+ // Overwriting the coordinates saves about 10% at runtime for larger
+ // shadings.
+ state->moveTo(0., 0.);
+ state->lineTo(1., 0.);
+ state->lineTo(0., 1.);
+ state->closePath();
+
+ GfxState::ReusablePathIterator *reusablePath = state->getReusablePath();
+
+// Idea:
+// - state->getReferenceToReusablePath()
+// the return value has the setX/setY methods, the standard GfxState doesn't
+// - state should not delete the current path until
+// - state->freeReusablePath()
+// but that's overhead and I am not highly motivated.
+//
+
+ if (shading->isParameterized()) {
+ // work with parameterized values:
+ double color0, color1, color2;
+ // a relative threshold:
+ const double refineColorThreshold = gouraudParameterizedColorDelta *
+ (shading->getParameterDomainMax() - shading->getParameterDomainMin());
+ for (i = 0; i < shading->getNTriangles(); ++i) {
+ shading->getTriangle(i, &x0, &y0, &color0,
+ &x1, &y1, &color1,
+ &x2, &y2, &color2);
+ gouraudFillTriangle(x0, y0, color0, x1, y1, color1, x2, y2, color2, refineColorThreshold, 0, shading, reusablePath);
+ }
+
+ } else {
+ // this always produces output -- even for parameterized ranges.
+ // But it ignores the parameterized color map (the function).
+ //
+ // Note that using this code in for parameterized shadings might be
+ // correct in circumstances (namely if the function is linear in the actual
+ // triangle), but in general, it will simply be wrong.
+ GfxColor color0, color1, color2;
+ for (i = 0; i < shading->getNTriangles(); ++i) {
+ shading->getTriangle(i, &x0, &y0, &color0,
+ &x1, &y1, &color1,
+ &x2, &y2, &color2);
+ gouraudFillTriangle(x0, y0, &color0, x1, y1, &color1, x2, y2, &color2, shading->getColorSpace()->getNComps(), 0, reusablePath);
+ }
+ }
+
+ delete reusablePath;
}
+
void Gfx::gouraudFillTriangle(double x0, double y0, GfxColor *color0,
double x1, double y1, GfxColor *color1,
double x2, double y2, GfxColor *color2,
- int nComps, int depth) {
+ int nComps, int depth, GfxState::ReusablePathIterator* path) {
double x01, y01, x12, y12, x20, y20;
GfxColor color01, color12, color20;
int i;
@@ -3114,15 +3165,17 @@ void Gfx::gouraudFillTriangle(double x0, double y0, GfxColor *color0,
}
}
if (i == nComps || depth == gouraudMaxDepth) {
+
state->setFillColor(color0);
out->updateFillColor(state);
- state->moveTo(x0, y0);
- state->lineTo(x1, y1);
- state->lineTo(x2, y2);
- state->closePath();
- if (!contentIsHidden())
- out->fill(state);
- state->clearPath();
+
+ path->reset(); assert(!path->isEnd());
+ path->setCoord(x0,y0); path->next(); assert(!path->isEnd());
+ path->setCoord(x1,y1); path->next(); assert(!path->isEnd());
+ path->setCoord(x2,y2); path->next(); assert(!path->isEnd());
+ path->setCoord(x0,y0); path->next(); assert( path->isEnd());
+
+ out->fill(state);
} else {
x01 = 0.5 * (x0 + x1);
y01 = 0.5 * (y0 + y1);
@@ -3130,24 +3183,79 @@ void Gfx::gouraudFillTriangle(double x0, double y0, GfxColor *color0,
y12 = 0.5 * (y1 + y2);
x20 = 0.5 * (x2 + x0);
y20 = 0.5 * (y2 + y0);
- //~ if the shading has a Function, this should interpolate on the
- //~ function parameter, not on the color components
for (i = 0; i < nComps; ++i) {
- color01.c[i] = (color0->c[i] + color1->c[i]) / 2;
- color12.c[i] = (color1->c[i] + color2->c[i]) / 2;
- color20.c[i] = (color2->c[i] + color0->c[i]) / 2;
+ color01.c[i] = (color0->c[i] + color1->c[i]) / 2.;
+ color12.c[i] = (color1->c[i] + color2->c[i]) / 2.;
+ color20.c[i] = (color2->c[i] + color0->c[i]) / 2.;
}
gouraudFillTriangle(x0, y0, color0, x01, y01, &color01,
- x20, y20, &color20, nComps, depth + 1);
+ x20, y20, &color20, nComps, depth + 1, path);
gouraudFillTriangle(x01, y01, &color01, x1, y1, color1,
- x12, y12, &color12, nComps, depth + 1);
+ x12, y12, &color12, nComps, depth + 1, path);
gouraudFillTriangle(x01, y01, &color01, x12, y12, &color12,
- x20, y20, &color20, nComps, depth + 1);
+ x20, y20, &color20, nComps, depth + 1, path);
gouraudFillTriangle(x20, y20, &color20, x12, y12, &color12,
- x2, y2, color2, nComps, depth + 1);
+ x2, y2, color2, nComps, depth + 1, path);
+ }
+}
+void Gfx::gouraudFillTriangle(double x0, double y0, double color0,
+ double x1, double y1, double color1,
+ double x2, double y2, double color2,
+ double refineColorThreshold, int depth, GfxGouraudTriangleShading* shading, GfxState::ReusablePathIterator* path ) {
+ const double meanColor = (color0 + color1 + color2) / 3;
+
+ const bool isFineEnough =
+ fabs(color0 - meanColor) < refineColorThreshold &&
+ fabs(color1 - meanColor) < refineColorThreshold &&
+ fabs(color2 - meanColor) < refineColorThreshold;
+
+ if (isFineEnough || depth == gouraudMaxDepth) {
+ GfxColor color;
+
+ shading->getParameterizedColor(meanColor, &color);
+ state->setFillColor(&color);
+ out->updateFillColor(state);
+
+ path->reset(); assert(!path->isEnd());
+ path->setCoord(x0,y0); path->next(); assert(!path->isEnd());
+ path->setCoord(x1,y1); path->next(); assert(!path->isEnd());
+ path->setCoord(x2,y2); path->next(); assert(!path->isEnd());
+ path->setCoord(x0,y0); path->next(); assert( path->isEnd());
+
+ out->fill(state);
+ } else {
+ double x01, y01, x12, y12, x20, y20;
+ double color01, color12, color20;
+ x01 = 0.5 * (x0 + x1);
+ y01 = 0.5 * (y0 + y1);
+ x12 = 0.5 * (x1 + x2);
+ y12 = 0.5 * (y1 + y2);
+ x20 = 0.5 * (x2 + x0);
+ y20 = 0.5 * (y2 + y0);
+ color01 = (color0 + color1) / 2.;
+ color12 = (color1 + color2) / 2.;
+ color20 = (color2 + color0) / 2.;
+ ++depth;
+ gouraudFillTriangle(x0, y0, color0,
+ x01, y01, color01,
+ x20, y20, color20,
+ refineColorThreshold, depth, shading, path);
+ gouraudFillTriangle(x01, y01, color01,
+ x1, y1, color1,
+ x12, y12, color12,
+ refineColorThreshold, depth, shading, path);
+ gouraudFillTriangle(x01, y01, color01,
+ x12, y12, color12,
+ x20, y20, color20,
+ refineColorThreshold, depth, shading, path);
+ gouraudFillTriangle(x20, y20, color20,
+ x12, y12, color12,
+ x2, y2, color2,
+ refineColorThreshold, depth, shading, path);
}
}
+
void Gfx::doPatchMeshShFill(GfxPatchMeshShading *shading) {
int start, i;
diff --git a/poppler/Gfx.h b/poppler/Gfx.h
index f1a8964..604f97f 100644
--- a/poppler/Gfx.h
+++ b/poppler/Gfx.h
@@ -299,7 +299,11 @@ private:
void gouraudFillTriangle(double x0, double y0, GfxColor *color0,
double x1, double y1, GfxColor *color1,
double x2, double y2, GfxColor *color2,
- int nComps, int depth);
+ int nComps, int depth, GfxState::ReusablePathIterator* path);
+ void gouraudFillTriangle(double x0, double y0, double color0,
+ double x1, double y1, double color1,
+ double x2, double y2, double color2,
+ double refineColorThreshold, int depth, GfxGouraudTriangleShading* shading, GfxState::ReusablePathIterator* path);
void doPatchMeshShFill(GfxPatchMeshShading *shading);
void fillPatch(GfxPatch *patch, int nComps, int depth);
void doEndPath();
diff --git a/poppler/GfxState.cc b/poppler/GfxState.cc
index 12e14d6..14e02fc 100644
--- a/poppler/GfxState.cc
+++ b/poppler/GfxState.cc
@@ -3362,6 +3362,8 @@ void GfxGouraudTriangleShading::getTriangle(
double out[gfxColorMaxComps];
int v, j;
+ assert(!isParameterized());
+
v = triangles[i][0];
*x0 = vertices[v].x;
*y0 = vertices[v].y;
@@ -3406,6 +3408,40 @@ void GfxGouraudTriangleShading::getTriangle(
}
}
+void GfxGouraudTriangleShading::getParameterizedColor(double t, GfxColor *color) {
+ double out[gfxColorMaxComps];
+
+ for (int j = 0; j < nFuncs; ++j) {
+ funcs[j]->transform(&t, &out[j]);
+ }
+ for (int j = 0; j < gfxColorMaxComps; ++j) {
+ color->c[j] = dblToCol(out[j]);
+ }
+}
+
+void GfxGouraudTriangleShading::getTriangle(
+ int i,
+ double *x0, double *y0, double *color0,
+ double *x1, double *y1, double *color1,
+ double *x2, double *y2, double *color2) {
+ int v;
+
+ assert(isParameterized());
+
+ v = triangles[i][0];
+ *x0 = vertices[v].x;
+ *y0 = vertices[v].y;
+ *color0 = colToDbl(vertices[v].color.c[0]);
+ v = triangles[i][1];
+ *x1 = vertices[v].x;
+ *y1 = vertices[v].y;
+ *color1 = colToDbl(vertices[v].color.c[0]);
+ v = triangles[i][2];
+ *x2 = vertices[v].x;
+ *y2 = vertices[v].y;
+ *color2 = colToDbl(vertices[v].color.c[0]);
+}
+
//------------------------------------------------------------------------
// GfxPatchMeshShading
//------------------------------------------------------------------------
@@ -4519,6 +4555,51 @@ void GfxPath::offset(double dx, double dy) {
//------------------------------------------------------------------------
// GfxState
//------------------------------------------------------------------------
+GfxState::ReusablePathIterator::ReusablePathIterator( GfxPath *path )
+ :
+ path(path),
+ subPathOff(0),
+ coordOff(0),
+ numCoords(0),
+ curSubPath(NULL)
+{
+ if( path->getNumSubpaths() ) {
+ curSubPath = path->getSubpath(subPathOff);
+ numCoords = curSubPath->getNumPoints();
+ }
+}
+
+bool
+GfxState::ReusablePathIterator::isEnd() {
+ return coordOff >= numCoords;
+}
+
+void
+GfxState::ReusablePathIterator::next() {
+ ++coordOff;
+ if( coordOff == numCoords ) {
+ ++subPathOff;
+ if( subPathOff < path->getNumSubpaths() ) {
+ coordOff=0;
+ curSubPath = path->getSubpath(subPathOff);
+ numCoords = curSubPath->getNumPoints();
+ }
+ }
+}
+
+void
+GfxState::ReusablePathIterator::setCoord( double x, double y ) {
+ curSubPath->setX( coordOff, x );
+ curSubPath->setY( coordOff, y );
+}
+
+void
+GfxState::ReusablePathIterator::reset() {
+ coordOff = 0;
+ subPathOff = 0;
+ curSubPath = path->getSubpath(0);
+ numCoords = curSubPath->getNumPoints();
+}
GfxState::GfxState(double hDPIA, double vDPIA, PDFRectangle *pageBox,
int rotateA, GBool upsideDown) {
diff --git a/poppler/GfxState.h b/poppler/GfxState.h
index 7dccfd5..d647f98 100644
--- a/poppler/GfxState.h
+++ b/poppler/GfxState.h
@@ -35,6 +35,8 @@
#include "Object.h"
#include "Function.h"
+#include <assert.h>
+
class Array;
class Gfx;
class GfxFont;
@@ -876,10 +878,37 @@ public:
virtual GfxShading *copy();
int getNTriangles() { return nTriangles; }
+
+ bool isParameterized() const { return nFuncs > 0; }
+
+ /**
+ * @precondition isParameterized() == true
+ */
+ double getParameterDomainMin() const { assert(isParameterized()); return funcs[0]->getDomainMin(0); }
+
+ /**
+ * @precondition isParameterized() == true
+ */
+ double getParameterDomainMax() const { assert(isParameterized()); return funcs[0]->getDomainMax(0); }
+
+ /**
+ * @precondition isParameterized() == false
+ */
void getTriangle(int i, double *x0, double *y0, GfxColor *color0,
double *x1, double *y1, GfxColor *color1,
double *x2, double *y2, GfxColor *color2);
+ /**
+ * Variant for functions.
+ *
+ * @precondition isParameterized() == true
+ */
+ void getTriangle(int i, double *x0, double *y0, double *color0,
+ double *x1, double *y1, double *color1,
+ double *x2, double *y2, double *color2);
+
+ void getParameterizedColor(double t, GfxColor *color);
+
private:
GfxGouraudVertex *vertices;
@@ -1002,6 +1031,9 @@ public:
double getY(int i) { return y[i]; }
GBool getCurve(int i) { return curve[i]; }
+ void setX(int i, double a) { x[i] = a; }
+ void setY(int i, double a) { y[i] = a; }
+
// Get last point.
double getLastX() { return x[n-1]; }
double getLastY() { return y[n-1]; }
@@ -1096,6 +1128,60 @@ private:
class GfxState {
public:
+ /**
+ * When GfxState::getReusablePath() is invoked, the currently active
+ * path is taken per reference and its coordinates can be re-edited.
+ *
+ * A ReusablePathIterator is intented to reduce overhead when the same
+ * path type is used a lot of times, only with different coordinates. It
+ * allows just to update the coordinates (occuring in the same order as
+ * in the original path).
+ */
+ class ReusablePathIterator {
+ GfxPath *path;
+ int subPathOff;
+
+ int coordOff;
+ int numCoords;
+
+ GfxSubpath *curSubPath;
+ public:
+ /**
+ * Creates the ReusablePathIterator. This should only be done from
+ * GfxState::getReusablePath().
+ *
+ * @param path the path as it is used so far. Changing this path,
+ * deleting it or starting a new path from scratch will most likely
+ * invalidate the iterator (and may cause serious problems). Make
+ * sure the path's memory structure is not changed during the
+ * lifetime of the ReusablePathIterator.
+ */
+ ReusablePathIterator( GfxPath* path );
+
+ /**
+ * Returns true if and only if the current iterator position is
+ * beyond the last valid point.
+ *
+ * A call to setCoord() will be undefined.
+ */
+ bool isEnd();
+
+ /**
+ * Advances the iterator.
+ */
+ void next();
+
+ /**
+ * Updates the coordinates associated to the current iterator
+ * position.
+ */
+ void setCoord( double x, double y );
+
+ /**
+ * Resets the iterator.
+ */
+ void reset();
+ };
// Construct a default GfxState, for a device with resolution <hDPI>
// x <vDPI>, page box <pageBox>, page rotation <rotateA>, and
@@ -1274,6 +1360,7 @@ public:
// Misc
GBool parseBlendMode(Object *obj, GfxBlendMode *mode);
+ ReusablePathIterator *getReusablePath() { return new ReusablePathIterator(path); }
private:
double hDPI, vDPI; // resolution
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
