Created attachment 22124 [details] [review] changes to poppler based on 0.10.3 as zip file 1. I implemented colorizing text in pattern colorspace. The trick is to use the text as clipping path and fill this path later with the existing methods. In the attached code it is implemented in SplashOutputDev, but it is very easy to implement it in other output devices, too. I.e. I already implemented in PSOutputDev, but I did that on base of the xpdf code, where I already made a lot of other changes, so I first have to examine, if all my changed fit into poppler, too. 2. I solved several bugs in blending and in transparency groups when using SPLASH_CMYK. I need SPLASH_CMYK to produce a bitmap and then convert this bitmap later to RGB using a cms engine like little cms. In case of blending an CMYK I convert the pixel first to RGB, use the existing and working routines for RGB and then convert the pixel back. This is not the best way, but I found no algorithm i.e. multiplying cmyk colors. If anyone has a better way, that would be very fine. 3. For a complete list of all my changes see changes.txt
Created attachment 22125 [details] 1. sample where colorizing text works
Created attachment 22126 [details] Sample for solved bugs with SPLASH_CMYK
Thanks for the code! But sending a zip file with all the changes files inside is quite difficult for us to work. Can you please attach a proper diff against the git master development tree or at least against poppler 0.10.3 instead of the full files? That will make easier for us to see the differences. If possible also provide separate patches for each change, i mean probably Lexer.cc: Helper for printing PDF commands. Useful to examine PDFs with deflated commands. Can go in a patch totally separated from the rest of the code. Keep in mind that reviewing changes is not easy and the smaller the patches the easier is letting them go inside the development tree.
Created attachment 22268 [details] [review] Helper for printing PDF commands. Useful to examine PDFs with deflated commands You think about something like this?
Created attachment 22273 [details] [review] In case of reallocating memory, set new allocated memory to default values Gfx.h 1. Remember some states for colorizing text in pattern colorspace GfxState.cc 1. In case of reallocating memory, set new allocated memory to default values. Solves problems at least under Solaris, and is the better way on all platforms. 2. Try to calculate the text bounding box. Not used at the moment. In PDFs I examined it is sufficient to use the clipping box already set for colorizing text in pattern colorspace
Comment on attachment 22273 [details] [review] In case of reallocating memory, set new allocated memory to default values I'm a little bit confused about the diff output from git. The diff I used says that the files are nearly identical
Created attachment 22274 [details] [review] support for SPLASH_CMYK in splashColorModeNComps
Created attachment 22275 [details] [review] colorizing text in pattern colorspace in SplashOutputDev
Created attachment 22276 [details] [review] colorizing text in pattern colorspace in PSOutputDev
(In reply to comment #3) > Thanks for the code! > > But sending a zip file with all the changes files inside is quite difficult for > us to work. > > Can you please attach a proper diff against the git master development tree or > at least against poppler 0.10.3 instead of the full files? > > That will make easier for us to see the differences. > > If possible also provide separate patches for each change, i mean probably > > Lexer.cc: Helper for printing PDF commands. Useful to examine PDFs with > deflated commands. > > Can go in a patch totally separated from the rest of the code. > > Keep in mind that reviewing changes is not easy and the smaller the patches the > easier is letting them go inside the development tree. > I attached now git diffs in 5 smaller pieces and add the changes for colorizing text in PSOutputDev, too. I removed the changes for color delta, because they are mainly part of a patch to pdftops, I will make a new bug report with a patch for color delta after these changes are committed. I didn't made a patch for the new files CIOutputDev.cc and CIOutputDev.h, but You're free to add them to the development tree. If You decide to do so, I will support them in futur, too. In the zip file there was on file missing, SplashTypes.h, needed for compiling, but it is part of the patches now.
Here are two PDF, which better show that colorizing text in pattern color space now works: a) see sms text on top http://www.alfa.de/de/admin/content/document/PDF/3907525_13_hi.pdf b) see magazin text on top http://www.alfa.de/de/admin/content/document/PDF/LCS_Asura.pdf
Some quick comments (i've not had a look at what you are doing, just a bit at general things): * Thanks again for the code * With patches it's much easier to review :-) * I see you seem to work in a publish company or something similar, are you aware of the licensing of poppler and what it means and are you sure you can contribute the code under that license? * patch from comment #4 looks ok * patch from comment #4 memsetting looks like a workaround for a problem somewhere else in the code, have you tried removing the code and passing the program though valgrind with the problematic pdf to see if it reports anything? * patch from comment #4 if you don't use the code at the moment i'll prefer leaving it out * patch from comment #5 is the same than patch from comment #4 * In patch from comment #8 I'd say that in Gfx::opEndText you are missing a delete colorSpaceText; before colorSpaceText = NULL; * In patch from comment #8: we don't use exceptions in poppler so that code has to be removed/reworked * In patch from comment #9 I guess there's a void PSOutputDev::beginTextObject(GfxState *state); missing in PSOutputDev.h * What is CIOutputDev for?
I can't count * patch from comment #4 memsetting looks like a workaround for a problem somewhere else in the code, have you tried removing the code and passing the program though valgrind with the problematic pdf to see if it reports anything? * patch from comment #4 if you don't use the code at the moment i'll prefer leaving it out * patch from comment #5 is the same than patch from comment #4 should be * patch from comment #5 memsetting looks like a workaround for a problem somewhere else in the code, have you tried removing the code and passing the program though valgrind with the problematic pdf to see if it reports anything? * patch from comment #5 if you don't use the code at the moment i'll prefer leaving it out * patch from comment #6 is the same than patch from comment #5
Some more comments: state->setRender(7); <-- sucks a bit state->setRender(7 /* Set foo bar */); <-- would be much more nice :-) I see that in Gfx::opEndText you do out->restoreState(state); but you are also doing it in SplashOutputDev::endTextObject that also gets called by Gfx::opEndText so i'm not sure if that's really needed, also might be that i'm not totally understanding what the code does :D Waiting for some feedback on your side before proceeding.
> * In patch from comment #8 I'd say that in Gfx::opEndText you are missing a > delete colorSpaceText; > before > colorSpaceText = NULL; I don't think so: state->setFillColorSpace(colorSpaceText); will leave it up to GfxState.cc to delete colorSpaceText next time the fill colorspace will be changed... > * In patch from comment #8: we don't use exceptions in poppler so that code > has to be removed/reworked Okay, I need that to find the three bugs in case of SPLASH_CMYK (wrong blending, wrong offset in setSoftMask, missing enum in splashColorModeNComps), to look at the bitmaps and therefore it is only inside a compiler switch, should not be part of released code. > * In patch from comment #9 I guess there's a > void PSOutputDev::beginTextObject(GfxState *state); > missing in PSOutputDev.h Yes, You're true. I merged the code from my changes in xpdf and just looked, if I'm able to compile it... > * What is CIOutputDev for? > I need to get some information about the pdf, at least what colorspaces the pdf is using, to decide, wether to create a monochrome, grayscale, rgb or cmyk bitmap. It is not sufficient to walk over the objects and look which Devices they are using, because it is allowed to use CS inside every PDF object. So its a helper device for ColorInfo.
(In reply to comment #14) > Some more comments: > state->setRender(7); <-- sucks a bit > state->setRender(7 /* Set foo bar */); <-- would be much more nice :-) Okay. The trick is to use the text as clipping path and fill it later on with the appropriate colorspace. Therefore I need here to set the render at least to 4. > > I see that in > Gfx::opEndText > you do > out->restoreState(state); > but you are also doing it in SplashOutputDev::endTextObject that also gets > called by Gfx::opEndText so i'm not sure if that's really needed, also might be > that i'm not totally understanding what the code does :D restoreState in SplashOutputDev::endTextObject will be done only if something goes wrong: if state->getFillColorSpace()->getMode() != csPattern at end of the text, no filling could be done, but at start of text the state was saved. So we have to restore it either in this case. I had that with one of the pdfs I load up: inside setting the text colorspace and fill color was already changed for later use! That was the reason why I remember this change of colorspace in colorSpaceText but will not submit that immediately but after successfully fill the text.
(In reply to comment #13) > I can't count > > * patch from comment #4 memsetting looks like a workaround for a problem > somewhere else in the code, have you tried removing the code and passing the > program though valgrind with the problematic pdf to see if it reports anything? I'm not sure if You're really true. If You look into the code for GfxGouraudTriangleShading the number of components is taken from the "Decode" dict: nComps = i; Therefore later on in this function only this number of components are filled for (i = 0; i < nComps; ++i) { verticesA[nVerticesA].color.c[i] = dblToCol(cMin[i] + cMul[i] * (double)c[i]); } The other components up to 4 are leaved unchanged, but greallocn will not initialize these values, at least not with gcc on Solaris. But later on, when the triangles are calculated and filled, all four color components will be examined. (It looks fine under windows!) > * patch from comment #4 if you don't use the code at the moment i'll prefer > leaving it out > * patch from comment #5 is the same than patch from comment #4 > > should be > > * patch from comment #5 memsetting looks like a workaround for a problem > somewhere else in the code, have you tried removing the code and passing the > program though valgrind with the problematic pdf to see if it reports anything? > * patch from comment #5 if you don't use the code at the moment i'll prefer > leaving it out > * patch from comment #6 is the same than patch from comment #5 comment #5 and comment #6 are the same code. I agree, You can leave the changes out. I started with this code, trying to calculate the text bounding box for later use as clipping box to fill it with the appropiate colorspace. But I wasn't really sure if my calculation was good enough, and in all my samples there was a clipping box set up in the PDF I can use. So leaving that code in could be confusing for other users. If I'll get a PDF where it would be necessary to use a text clipping box again I can insert the code once again and use the box at the appropriate places...
Ok, almost everything's clear know, what about the licensing? Is it ok for you to contribute this patch under the GPLv2 or later license?
(In reply to comment #18) > Ok, almost everything's clear know, what about the licensing? Is it ok for you > to contribute this patch under the GPLv2 or later license? > Sorry, I missed that: Of course, if I upload patches, it is ok for me and my company to contribute it under GPLv2 or later license.
The patch causes a rendering regression on http://www.umweltbundesamt.org/fpdf-l/2826.pdf page 24 the one with a bar graph that says Seite 19 on top right. Can you try to fix this?
(In reply to comment #20) > The patch causes a rendering regression on > http://www.umweltbundesamt.org/fpdf-l/2826.pdf page 24 the one with a bar graph > that says Seite 19 on top right. Can you try to fix this? > Thank You for providing that example. If You mean with "rendering regression" that the text "Gesamtbewertung Schadstoffe" is missing: I solved that now. Perhaps You now can understand my "security line" in endTextObject: when colorspace changed during rendering the text from pattern colorspace to another, I can't not fill the text anymore. Therefore I have to do something special if state->getFillColorSpace()->getMode() != csPattern now. But the implemention was neither in PSOutputDev nor in SplashOutputDev good enough, and I couldn't test that because I haven't any sample. BTW, it could also be the other way round: if text rendering is started but the colorspace is changed to pattern colorspace before setting the text, I don't recognize that, and the text will be placed solid like it was done before. Once again: because I have no sample with such a case, I can not test and therefore it's not easy to implement that. Unfortunately I can't not provide a good diff, because I already implemented imagemasks in pattern colorspace. So I just insert the new implementation for endTextObject in both cases, You'll see there is not a big difference: void SplashOutputDev::endTextObject(GfxState *state) { if (haveCSPattern) { state->setRender(savedRender); haveCSPattern = gFalse; if (state->getFillColorSpace()->getMode() != csPattern) { if (textClipPath) { splash->fill(textClipPath, gTrue); delete textClipPath; textClipPath = NULL; } restoreState(state); updateFillColor(state); } } if (textClipPath) { splash->clipToPath(textClipPath, gFalse); delete textClipPath; textClipPath = NULL; } } and void PSOutputDev::endTextObject(GfxState *state) { if (haveCSPattern) { writePS("Tclip*\n"); haveTextClip = gFalse; state->setRender(savedRender); haveCSPattern = gFalse; if (state->getFillColorSpace()->getMode() != csPattern) { double cxMin, cyMin, cxMax, cyMax; state->getClipBBox(&cxMin, &cyMin, &cxMax, &cyMax); writePSFmt("{0:.4g} {1:.4g} {2:.4g} {3:.4g} re\n", cxMin, cyMin, cxMax, cyMax); writePS("f*\n"); restoreState(state); updateFillColor(state); } } else if (haveTextClip) { writePS("Tclip\n"); haveTextClip = gFalse; } }
I found another regression with your patch, second page of http://bugs.kde.org/attachment.cgi?id=8847 crashes with your patch
(In reply to comment #22) > I found another regression with your patch, second page of > http://bugs.kde.org/attachment.cgi?id=8847 crashes with your patch Congratulation: It seems as if You have found a PDF, where text rendering is started in a normal pattern space but the colorspace is changed to pattern colorspace before setting the text. I have to take a deeper look into it, at the moment I haven't the atual code at home. But it seems to be sufficient to change only this line: void Gfx::opEndText(Object args[], int numArgs) { out->endTextObject(state); --> if (out->supportTextCSPattern(state) && textHaveCSPattern) { ... If this helps, I would like to fix this problem (Firefox 1.0 should be displayed in pattern color space and not with solid color) in a additional patch!
(In reply to comment #23) > If this helps, I would like to fix this problem (Firefox 1.0 should be > displayed in pattern color space and not with solid color) in a additional > patch! > It was quite easy to solve, and even if firefox 1.0 should be no more supported :D, I attach an additional patch for Gfx.cc and Gfx.h. The diff already contains also the changes for colorizing image masks in pattern colorspace, therefore I replace the patch in that bug, too. If You're only interested in the changes for text, just search for "drawText" and fill in that changes!
Created attachment 22703 [details] [review] changes for display firefox correctly
(In reply to comment #13) > * patch from comment #4 memsetting looks like a workaround for a problem > somewhere else in the code, have you tried removing the code and passing the > program though valgrind with the problematic pdf to see if it reports anything? Here is a sample where at least on Solaris without memsetting will give a terrible result: http://www.alfa.de/de/admin/content/document/PDF/Renault_49.pdf Look at the renault logo in the lower left corner!
Hi Thomas, sorry for taking so much to answer back, i've been incredibly busy, could you please repost a patch with all the needed changes for this bug and for 19994? I'll pass it through my test suite and if there's no regression i'll commit it to the repo. Sorry again for being so slow.
Created attachment 24746 [details] [review] Complete patch 19670 and 19994 I've made an actual and complte diff for 19670 and 19994. There are some smaller changes between this and the old patch, i.e. I recognized that splashOutBlendDifference didn't work correctly in case of SPLASH_CMYK, but all these changes are for case of SPLASH_CMYK! Hope it helps! Best regards, Thomas
Hi Thomas, the file at https://bugs.freedesktop.org/show_bug.cgi?id=13639 presents regressions with your patch, for example in page one the text "Programas de CAD" should be red and is not with your patch applied.
Another regression, "Guarded Area" text of page 148 of http://launchpadlibrarian.net/14697902/13902x.pdf disappears when using your patch
Another regression of text color in second page of https://bugs.freedesktop.org/attachment.cgi?id=21993
Another regression of text color in second page of https://bugs.freedesktop.org/attachment.cgi?id=22266
http://www.dkuug.dk/jtc1/sc2/wg2/docs/n3580.pdf also has the text regression problem on lots of pages (3, 4, 5, 7, 9, etc), moreover the document renders much more slowly with this patch than without 23 minutes vs 2 minutes
on a side note it fixes 3 pdf i had in my test suite, so it's definitely a step in the good direction if you can fix these regressions :-)
Created attachment 25412 [details] [review] changes in Gfx.cc To solve the regressions, I made two changes, to show the only differences here the two changed methods: In opSetFillGray I removed my changes. I definitely need this changes in opSetFillCMYKColor, but they are wrong in opSetFillGray: void Gfx::opSetFillGray(Object args[], int numArgs) { GfxColor color; state->setFillPattern(NULL); state->setFillColorSpace(new GfxDeviceGrayColorSpace()); out->updateFillColorSpace(state); color.c[0] = dblToCol(args[0].getNum()); state->setFillColor(&color); out->updateFillColor(state); } In opSetFillColorSpace I need to recognize if I'm in text drawing and, if so, the colorspace change from / to csPattern: void Gfx::opSetFillColorSpace(Object args[], int numArgs) { Object obj; GfxColorSpace *colorSpace; GfxColor color; state->setFillPattern(NULL); res->lookupColorSpace(args[0].getName(), &obj); if (obj.isNull()) { colorSpace = GfxColorSpace::parse(&args[0]); } else { colorSpace = GfxColorSpace::parse(&obj); } obj.free(); if (colorSpace) { state->setFillColorSpace(colorSpace); out->updateFillColorSpace(state); colorSpace->getDefaultColor(&color); state->setFillColor(&color); out->updateFillColor(state); if (drawText) { if (colorSpace->getMode() == csPattern) { colorSpaceText = NULL; textHaveCSPattern = gTrue; out->beginTextObject(state); } else if (textHaveCSPattern) { out->endTextObject(state); doPatternFill(gTrue); out->restoreState(state); textHaveCSPattern = gFalse; out->beginTextObject(state); } } } else { error(getPos(), "Bad color space (fill)"); } } I attach a new diff for Gfx.cc. I tested Your found regressions (also the speed should be okay now again) and my PDFs where I need the changes. These PDFs are working now!
Ok, i've finished the regression testing of SplashOutputDev and it's good. Next i'm going to test PSOutputDev next. Another problem is that we need to fix CairoOutputDev to support this feature too though I'm not sure if that's of your interest. Carlos if Thomas doesn't have time for adding the feature to CairoOutputDev do you think you or any of the Cairo guys can add support for it? Page 5 of bug #2807 is clear example of this feature.
Created attachment 25792 [details] [review] cairo patch Patch for pattern text support for CairoOutputDev
Thanks Adrian, i'l regtest it after PS.
Thomas, i've found some problems in the PS generation (the generated PS "crash" when run through gs) Invalid PS files are generated in http://bugs.kde.org/attachment.cgi?id=15494 http://bugs.kde.org/attachment.cgi?id=26258 http://bugs.freedesktop.org/attachment.cgi?id=11319 And some others, can you try to fix these regressions?
(In reply to comment #39) > Thomas, i've found some problems in the PS generation (the generated PS "crash" > when run through gs) > > Invalid PS files are generated in > http://bugs.kde.org/attachment.cgi?id=15494 > http://bugs.kde.org/attachment.cgi?id=26258 > http://bugs.freedesktop.org/attachment.cgi?id=11319 > > And some others, can you try to fix these regressions? > Albert, sorry for the inconvenience. I'm still using my modified PSOutputDev from xpdf, so probably I've made some mistakes merging the code. At the moment I haven't the time to have a deeper look into it, I'll do that next weekend. In the meantime, could You please provide me some additional information? 1. How Do You produce the PostScript? Do You use pdftops inside poppler, and if so, what cmdline parameters do You use? If not, which values are You using to setup GlobalParams? 2. What version of gs are You using? Is there some error output from gs? 3. Can You provide a PS-file wich crashes? Perhaps I can find the problem just looking into the produced PS.
1. i'm using pdftops with no extra parameters 2. i'm using gs 8.64 3. Yep, attaching it in a moment
Created attachment 25920 [details] ps that presents a regression This file (output of http://bugs.kde.org/attachment.cgi?id=15494) gives this error Error: /nocurrentpoint in --rmoveto-- Operand stack: 0 0 0.0 0.0 32 0.00210935 0.0 (AM) 0.0 0.0 Execution stack: %interp_exit .runexec2 --nostringval-- --nostringval-- --nostringval-- 2 %stopped_push --nostringval-- --nostringval-- --nostringval-- false 1 %stopped_push 1862 1 3 %oparray_pop 1861 1 3 %oparray_pop 1845 1 3 %oparray_pop 1739 1 3 %oparray_pop --nostringval-- %errorexec_pop .runexec2 --nostringval-- --nostringval-- --nostringval-- 2 %stopped_push --nostringval-- --nostringval-- Dictionary stack: --dict:1147/1684(ro)(G)-- --dict:0/20(G)-- --dict:70/200(L)-- --dict:67/75(L)-- --dict:18/25(L)--
Thanks, it helps, and I think I was able to solve it. It is a problem in Gfx::opSetFillColorSpace, in case that text colorspace change from a pattern colorspace to a solid colorspace: 1. doPatternFill(gTrue) already restores the state in OutputDev. 2. In a stateless OutputDevice, after restoring the state, we need to restore some text parameters. Therefore the solution is: void Gfx::opSetFillColorSpace(Object args[], int numArgs) { Object obj; GfxColorSpace *colorSpace; GfxColor color; state->setFillPattern(NULL); res->lookupColorSpace(args[0].getName(), &obj); if (obj.isNull()) { colorSpace = GfxColorSpace::parse(&args[0]); } else { colorSpace = GfxColorSpace::parse(&obj); } obj.free(); if (colorSpace) { state->setFillColorSpace(colorSpace); out->updateFillColorSpace(state); colorSpace->getDefaultColor(&color); state->setFillColor(&color); out->updateFillColor(state); if (drawText) { if (colorSpace->getMode() == csPattern) { colorSpaceText = NULL; textHaveCSPattern = gTrue; out->beginTextObject(state); } else if (textHaveCSPattern) { out->endTextObject(state); doPatternFill(gTrue); //out->restoreState(state); out->beginTextObject(state); out->updateRender(state); out->updateTextMat(state); out->updateTextPos(state); textHaveCSPattern = gFalse; } } } else { error(getPos(), "Bad color space (fill)"); } } I tested it with the PDFs of comment 39, and it works well. Unfortunately I'm not able to test it at home with SplashOutputDev again, I'll do that tomorrow, but because the three new functions calls are not specified there (so they are dummy in SplashOutputDev, the changed code should not run into new problems there.
(In reply to comment #43) > I tested it with the PDFs of comment 39, and it works well. Unfortunately I'm > not able to test it at home with SplashOutputDev again, I'll do that tomorrow, > but because the three new functions calls are not specified there (so they are > dummy in SplashOutputDev, the changed code should not run into new problems > there. > Sorry, I was very busy yesterday, so I just make the tests (and looked into it with the debugger): As I hoped, there are no significant changes in SplashOutputDev, and therefore it still works. Do You want a final patch?
Not for now, let me re-run the regression testing. /me needs a faster computer
Still two small bugs in pdftops: * First one is text becomes "malformed" in page 2 of http://bugs.freedesktop.org/attachment.cgi?id=21993 and in page 148 of http://bugs.freedesktop.org/show_bug.cgi?id=16094 * Second one is the generated ps crashes gs on page 3 of http://www.dkuug.dk/jtc1/sc2/wg2/docs/n3580.pdf We are getting close!
Created attachment 26096 [details] Another regression This time I found a regression by my own, both in PSOutputDev and SplashOutputDev, with my patch. The PDF doesn't look well without my patch, but it still runs into a problem with my patch
(In reply to comment #46) > We are getting close! > I fear, You're wrong :-(. I changed opSetFillGray in comment 35, to solve the same regressions in SplashOutputDev, but this runs now into problems with PSOutputDev. I could fix it in this function in the following way: void Gfx::opSetFillGray(Object args[], int numArgs) { GfxColor color; if (textHaveCSPattern) { out->endTextObject(state); out->restoreState(state); state->setFillPattern(NULL); state->setFillColorSpace(new GfxDeviceGrayColorSpace()); out->updateFillColorSpace(state); color.c[0] = dblToCol(args[0].getNum()); state->setFillColor(&color); out->updateFillColor(state); out->beginTextObject(state); out->updateRender(state); out->updateTextMat(state); out->updateTextPos(state); textHaveCSPattern = gFalse; } else { state->setFillPattern(NULL); state->setFillColorSpace(new GfxDeviceGrayColorSpace()); out->updateFillColorSpace(state); color.c[0] = dblToCol(args[0].getNum()); state->setFillColor(&color); out->updateFillColor(state); } } but because of the regression in comment 47 and because this will not work if text is already outputted before calling this method I had a deeper look into it and found a solution. But: this means a complete new patch, and before I upload the new patch, I'll run it over my "testsuite"
Created attachment 26121 [details] [review] Replace patch of #comment 28 Now that my changes are running through "my testsuite", as I already mentioned, the changes are too big for just talking about code changes. Therefore I attach an actual git diff. (I fear, I haven't the faster computer, but I have fewer PDFs to test with :-)). Is it possible for Adrian to have a second look into the new patch? I fear that Cairo will have simular problems... There are two main changes in SplashOutputDev: 1. the implementation for updateRender (new in SplachOutputDev) is just to keep care, that the text clipping path is NOT for using it to fill with pattern color space, but as a real clipping path, see PDF of comment #47 as sample (it is zipped!). 2. The new method deviceHasTextClip is to make the program faster, when no text is printed AND for avoiding regressions when doing pattern fill on no clipping path.
(In reply to comment #49) Oh, I forgot to tell: running the PDFs through my testsuite, I encountered an additional bug in PSOutputDev::setupImage, nothing to do with my patch, but I solved that also in the new patch. You can see that running pdftops over http://www.alfa.de/de/admin/content/document/PDF/cafebarspielsalon_26022009_01.pdf from Bug# 19994.
All pdf of my regression test worked :-) I'm about to commit it, but one last questions, in splashOutBlendColor you commented the //~ (0xff - ...) should be clipped part, what's the status of that part of code, should be removed? It's a todo? Adrian you should see latest comments about regression and check if they affect the cairo backend. Also i need a way to run the regression test against the cairo backend.
(In reply to comment #51) > All pdf of my regression test worked :-) Good news :-) > > I'm about to commit it, but one last questions, in splashOutBlendColor you > commented the //~ (0xff - ...) should be clipped part, what's the status of > that part of code, should be removed? It's a todo? > > You can remove the clipped part, it was the old code. The new code (insert lines before it) give a quite better output in case of SPLASH_CMYK. I'll plan to change the splashOutBlend-functions later on once again when support of little cms is completely part of poppler. I would prefer to use then little cms for this kind of color conversion, too.
Patch commited, i'm closing the bug. I'll open a new one for cairo backend. If you want to use lcms for the conversions, patches welcome, we already have an optional dependency on it ;-)
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.