Wt

Hi Bruce,

I already went ahead and applied the parseCssColor patches. I'm currently looking at the other patches. One thing I've noticed is that the setFillColor and setStrokeColor methods have HPDF_ExtGState as their return type, but they don't return anything?

Regards,

Roel

Hi Roel,

Thanks for the careful review. The return types for setFillColor and setStrokeColor should be changed to void. In an earlier version, these methods returned gstate for the caller's use. After some restructuring, the return values were no longer needed and I removed the return statements without updating the return types. If I had compiled with -Wall, I would have caught this.

Regards,

Bruce

I've noticed that reverting the change from if (flags & (Transform | Clipping)) to just if (flags) would already cause the widget gallery to work properly on Linux. Oddly enough, this does not affect the widget gallery on Windows, so I guess this is due to the font metrics (on Windows we use DirectWrite). I guess the unconditional transform change is causing issues on Linux. Applying this revert does not hurt, though, but I do wonder why that inserts so much space on Linux, and not Windows.

I guess the other part of your patches (the two maps) is intended to limit the amount of ExtGStates? On every change, a new ExtGState would be created, but now there's one for every stroke alpha that's used, and one for every fill alpha that's used. The code looks alright and seems to work alright, but I do wonder about the interaction of SetExtGState (the gs instruction), Page_GSave (the q instruction), and Page_GRestore (the Q instruction). Does SetExtGState push a new state onto the stack or not? It seems not, because it seems that we're relying on it not pushing a new state on the stack. I will do more testing and looking at documentation, but I was wondering if you maybe knew the answer to that question? I didn't write the transparency support, so I'm still a bit new to it.

Update: it looks like page 127 and 128 from this document: https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/PDF32000_2008.pdf suggest that the patches you provided are indeed correct.

Hi Roel,

Thanks for working through all of this. I should have included more comments with the patch. Here are some additional notes while the work is still fresh in my mind...

1. The extended gstate is dictionary-based (somewhat like fonts), as described in the document link you provided. I found this surprising, too.

2. There is a limit in libharu on dictionary entries (based on PDF spec). The 1007 error (HPDF_DICT_COUNT_ERR) that was getting logged in issue #6629 was a result of this. The two alpha maps in the patch are intended to limit the maximum number of extended gstate entries required for supporting transparency to a maximum of 512.

3. There is another issue related to when it is permissible to call HPDF_Page_SetExtGState. For instance, it cannot be called after HPDF_Page_BeginText. An attempt to do so results in a 1051 error (HPDF_PAGE_INVALID_GMODE). I had to work around this when adding transparency support to drawText.

4. I reverted to the earlier version of WPdfImage.C before fully understanding the issues. I thought the reverted changes were limited to transparency support.

Regards,

Bruce