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
diff --git a/poppler/Gfx.cc b/poppler/Gfx.cc
index 221caef..dbb8916 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
@@ -3087,15 +3095,39 @@ 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();
+
+ 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);
+ }
+
+ } else {
+ // this always produces output -- even for parameterized ranges.
+ // But it is wrong for parameterized colors:
+ 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);
+ }
}
}
@@ -3116,13 +3148,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();
+ GfxPath* path = state->getPath();
+ GfxSubpath* subpath = path->getSubpath(0);
+ subpath->setX(0, x0);
+ subpath->setY(0, y0);
+ subpath->setX(1, x1);
+ subpath->setY(1, y1);
+ subpath->setX(2, x2);
+ subpath->setY(2, y2);
+ subpath->setX(3, x0);
+ subpath->setY(3, y0);
+ out->fill(state);
} else {
x01 = 0.5 * (x0 + x1);
y01 = 0.5 * (y0 + y1);
@@ -3130,8 +3166,6 @@ 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;
@@ -3147,6 +3181,67 @@ void Gfx::gouraudFillTriangle(double x0, double y0, GfxColor *color0,
x2, y2, color2, nComps, depth + 1);
}
}
+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 ) {
+ 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);
+ GfxPath* path = state->getPath();
+ GfxSubpath* subpath = path->getSubpath(0);
+ subpath->setX(0, x0);
+ subpath->setY(0, y0);
+ subpath->setX(1, x1);
+ subpath->setY(1, y1);
+ subpath->setX(2, x2);
+ subpath->setY(2, y2);
+ subpath->setX(3, x0);
+ subpath->setY(3, y0);
+
+ 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);
+ gouraudFillTriangle(x01, y01, color01,
+ x1, y1, color1,
+ x12, y12, color12,
+ refineColorThreshold, depth, shading);
+ gouraudFillTriangle(x01, y01, color01,
+ x12, y12, color12,
+ x20, y20, color20,
+ refineColorThreshold, depth, shading);
+ gouraudFillTriangle(x20, y20, color20,
+ x12, y12, color12,
+ x2, y2, color2,
+ refineColorThreshold, depth, shading);
+ }
+}
+
void Gfx::doPatchMeshShFill(GfxPatchMeshShading *shading) {
int start, i;
diff --git a/poppler/Gfx.h b/poppler/Gfx.h
index f1a8964..a49c773 100644
--- a/poppler/Gfx.h
+++ b/poppler/Gfx.h
@@ -300,6 +300,10 @@ private:
double x1, double y1, GfxColor *color1,
double x2, double y2, GfxColor *color2,
int nComps, int depth);
+ 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);
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 fe3ee77..60bb421 100644
--- a/poppler/GfxState.cc
+++ b/poppler/GfxState.cc
@@ -3351,6 +3351,8 @@ void GfxGouraudTriangleShading::getTriangle(
double out[gfxColorMaxComps];
int v, j;
+ assert(!isParameterized());
+
v = triangles[i][0];
*x0 = vertices[v].x;
*y0 = vertices[v].y;
@@ -3395,6 +3397,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
//------------------------------------------------------------------------
diff --git a/poppler/GfxState.h b/poppler/GfxState.h
index 7dccfd5..4d226de 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]; }
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
