Bug 689931 - PDF Graphics state w/ SMask is not rendered correctly
Summary: PDF Graphics state w/ SMask is not rendered correctly
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: Color (show other bugs)
Version: master
Hardware: All All
: P3 major
Assignee: Michael Vrhel
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-01 08:42 UTC by Ray Johnston
Modified: 2009-04-28 14:40 UTC (History)
3 users (show)

See Also:
Customer:
Word Size: ---


Attachments
Acrobat_AR7_vs_GS.png (103.45 KB, image/png)
2008-07-01 08:43 UTC, Ray Johnston
Details
gstate_SMask_w_overlap.pdf (6.60 KB, application/pdf)
2008-07-01 08:44 UTC, Ray Johnston
Details
gstate_SMask_CMYK_overlap_BM_Screen.pdf (7.81 KB, application/pdf)
2008-09-04 00:16 UTC, Ray Johnston
Details
prelim_fix.pat (2.48 KB, patch)
2008-09-04 01:03 UTC, Ray Johnston
Details | Diff
second_prelim_fix.pat (5.11 KB, patch)
2008-09-10 12:22 UTC, Ray Johnston
Details | Diff
Smask_Mono_Patch.diff (7.14 KB, patch)
2008-09-11 13:12 UTC, Michael Vrhel
Details | Diff
SmaskPatch.diff (51.44 KB, patch)
2008-10-01 22:45 UTC, Michael Vrhel
Details | Diff
SmaskPatch2.diff (51.69 KB, patch)
2008-10-02 22:16 UTC, Michael Vrhel
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ray Johnston 2008-07-01 08:42:03 UTC
This was discovered while testing the 'simplified' test case (trani.pdf) of bug
688601. Excerpting from comment #17:

Our implementation of SMask does not allow for painting individual objects with
an SMask. The SMask in question is being ignored (it is stored in ctx->maskbuf,
but it is never used in subsequent pop_transparency_group operations).

I've constructed a new test case that shows the differences including the effect
when there are overlapping objects mentioned in the PDF spec:
   "Note: The current soft mask in the graphics state is intended to be used
    to clip only a single object at a time (either an elementary object or a
    transparency group). If a soft mask is applied when painting two or more
    overlapping objects, the effect of the mask multiplies with itself in the
    area of overlap (except in a knockout group), producing a result shape or
    opacity that is probably not what is intended. To apply a soft mask to
    multiple objects, it is usually best to define the objects as a
    transparency group (see Specifying SECTION 7.5 Transparency in PDF) and
    apply the mask to the group as a whole."

In order to see Cyan with Ghostscript instead of Black, the patch attached
to 688601 (http://bugs.ghostscript.com/attachment.cgi?id=4175&action=view)
needs to be applied, but this does not really affect the SMask issue.
Comment 1 Ray Johnston 2008-07-01 08:43:08 UTC
Created attachment 4176 [details]
Acrobat_AR7_vs_GS.png

Screen capture showing the difference.
Comment 2 Ray Johnston 2008-07-01 08:44:08 UTC
Created attachment 4177 [details]
gstate_SMask_w_overlap.pdf
Comment 3 Ray Johnston 2008-07-03 09:02:40 UTC
Glancing at the code in pdf_begin_transparency_mask() this seems incorrect
since it inserts a transparency group -- the final statement of function is:
    return  pdf_begin_transparency_group(pis, pdev, pparams);

I don't see how this will properly emulate the input PDF when an SMask is part
of a ExtGState set in the Content stream, but not grouped by the input PDF.

I am cc'ing Ken and Igor on this since this, and bug 688601 relate to the
pdfwrite handling of soft masks.
Comment 4 leonardo 2008-07-03 10:22:38 UTC
Bumpimng priority because it's a part of a runnin g project (the 
clist/transparency optimization).
Comment 5 Ray Johnston 2008-09-04 00:16:50 UTC
Created attachment 4358 [details]
gstate_SMask_CMYK_overlap_BM_Screen.pdf

This is a hand modified version of gstate_SMask_w_overlap.pdf to change:
 - CS for the SMask is CMYK instead of RGB
 - Coordinates and color of one of the rectangles in the SMask altered to
   show more complexity
 - BlendMode (BM) is 'Screen' instead of Normal.

This shows many anomalies in our rendering compared to Adobe Acrobat in RGB
mode (-dDisplayFormat=16#20804) and CMYK (-dDisplayFormat=16#20808) with
and without -dUseCIEColor.

To also see the effects with BM Normal, change line 187 in the file (this is
an 'inflated' PDF so it can be easily edited.

This should keep us busy for a bit while I work on a more complex example
where there is an SMask used in the SMask form.
Comment 6 Ray Johnston 2008-09-04 01:03:55 UTC
Created attachment 4360 [details]
prelim_fix.pat

These patches are preliminary changes that get us closer (but still not there)
on the test files.

1) Use 'q' ... 'Q' around the execution of the SMask form to prevent it from
changing elements of the outer form. See below about this with pdfwrite.
2) When SoftMask is not null, put each drawing operation (fill, stroke, image)
in a separate group to apply the current SMask. Not efficient and not complete
for text painting operations.
3) preserve the ExtGState, not just the gstate, using //nodict begin ... end
around the pre-processing of the SMask to get the CS (ColorSpace).

This approach still needs to be supplemented to work with pdfwrite by passing
the soft_mask_id of the gs_imager_state around the grestore when the SMask is
executed (the 'Q' operator) and also creating a separate group for text
operations when SoftMask is not null is not included.

Micahel Ralph and I have discussed these and they have these changes, but I
wanted to attach them here so they don't get lost and to make sure Michael and
I are 'on the same page'.
Comment 7 leonardo 2008-09-05 06:10:13 UTC
Since Henry asked me to share my ideas, here is one related to this bug. 

The original design of Ghostscript transparency is baased on assumption that 
the transparency stack is handled by the pdf14 device, which also performs the 
group raster buffering. When the pdf interpreter works for a high level 
device, there is no need to rasterize transparency groups, so pdf14 is not 
installed. The we have no transparency stack.

The right resolution should be moving the transparency stack to graphics 
library. Due to that soft mask id was added to the graphic state as a first 
step of "owning" the transparency data by the graphics library. This right 
architecture should be like this :

1. A graphic state points to a transparency stack element.
2. A new transparency stack element is always created with a new graphic 
state, and released with it.
3. Since operator gstate can clone imager state, transparency stack elements 
are reference counted.
4. A device may associate a transparency buffer with the graphic state.
5. A transparency buffer points to device which produced it.
6. The device points to the active transparency stack element, which is being 
rendered, and to a transparency buffer, which is curently used as a mask. 
7. Constraint : when a graphic state points to transparency stack element, 
setgstate is successful only if its argument points to same transparency stack 
element, which is active for the device, which is (indirectly) pointed by the 
graphic state argument. This constraint prevents migration of transparency 
buffers between devices, and keeps transparency buffers be consistent with 
current process colors of the device. "Indirectly" means "immediately or 
through a forwarding device such as clipper".
8. Implement object rasterization to the current transparency buffer *through* 
the current transparency mask, so that mask may be easy replaced or biased 
between objects or between glyphs.
9. implement associating of completed transparency buffers with a pattern as 
an image with soft mask, and extend the pattern p;ainting proc with such 
images. 
Comment 8 Ray Johnston 2008-09-10 12:22:13 UTC
Created attachment 4387 [details]
second_prelim_fix.pat

An almost complete patch

TODO: execute text painting operations within a transparency group if SoftMask
is not null

This patch executes the SMask group within a 'q' ... 'Q' (gsave //nodict begin
... end grestore) so that the SMask doesn't pollute the outer graphics state.
This is similar to the first patch attached to 688601, but it passes the
gs_imager_state->soft_mask_id around the grestore for pdfwrite. New PS
operators for .currentsoftmaskid and .setsoftmaskid are added to ztrans.c and
gstrans.[ch] to support this from the PDF interpreter.

Fills, strokes and images are executed in a transparency group if the operation
is not within a Form. This is the case when SoftMask is not null since it is
set to null when groups are executed (see .execgroup)

Also .execgroup now clears the path (newpath) prior to executing the group so
that the group doesn't inherit the path from the outer context. The 'q' ... 'Q'
equivalent save of the gstate and ExtGState are moved out of .execgroup to
avoid and extra save level when invoked from .execmaskgroup, so a save is now
added to .paintgroupform.
Comment 9 Ray Johnston 2008-09-10 12:26:28 UTC
Adding Alex since the pdf_***.ps files are his
Comment 10 Michael Vrhel 2008-09-11 13:12:42 UTC
Created attachment 4389 [details]
Smask_Mono_Patch.diff

This is a separate patch which addresses issues relating to properly computing
luminance of the Smask output.	When the SMask is popped, we now convert device
RGB or device CMYK values to luminance values.	This reduces memory use (if the
Smask had been RGB or CMYK) and now matches the Adobe output when we have a
device color path. The remaining part to do on this is to map to true luminance
(Y of XYZ) when we have a CIE color space.  That difference is relatively
minor.
Comment 11 leonardo 2008-09-22 01:05:48 UTC
Regarding patch 4387 : I think we should not complicate things with new ps 
operators. Instead I suggest to implement "gsave setcolorspace" in C inside 
zbegintransparencymaskgroup and "grestore" inside zendtransparencymask . The 
first became possible after implementing setcolorspace in C by Ken. This way 
better complies to handling the transparency stack by the graphics library.
Comment 12 Ray Johnston 2008-09-23 09:16:41 UTC
In response to the previous comment from Igor (#11)...

While it is possible to fix Alex's patch from 688601, attachment #4180 [details], by
adding "//nodict begin" BEFORE changing the colorspace, and adding an "end"
before returning, we still have a design issue being discussed about how to
prevent multiple collection of the SMask (a rather expensive process).

Exposing an 'id' for the SMask to the PDF interpreter requires at least the
.currentsoftmaskid operator, but as this patch is preliminary, I will leave
it up to Michael to determine if we can do this (prevent multiple collection
of the same SMask) some other way.

There are two cases where the same SMask can be collected more than once:
1) in the DoImage operation when the image does not have an SMask, but the
   graphics state does.
2) when more than one object or Form (transparency group) is painted with
   a gstate that has an SMask

The other part that would be needed is to modify the transparency buffering
logic so that the 'maskbuf' doesn't get discarded after every
'.endtransparencygroup' as it does now. This retention would also need a
inform the transparency buffer logic when the maskbuf is no longer needed.
The pdfwrite device does this by checking the soft_mask_id in the gstate
before every 'drawing' operation (gdevpdfg.c:pdf_update_alpha).

Comment 13 Michael Vrhel 2008-10-01 22:45:06 UTC
Created attachment 4458 [details]
SmaskPatch.diff

This patch is a rough beginning in getting proper output for softmasks.  This
provides proper rendering for the Smask with overlap example that is attached
to this bug.  The trans.pdf file that was in the original bug also renders
properly but there appears to be a memory issue when the output device is CMYK
without -dUseCIEColor.	The reclaim that occurs with debug set results in an
access violation at the end of the rendering.  The exact call is in
gc_validate_spaces.  It would be nice if I could force the reclaims from C-code
to help me track down when and where the problem is introduced.  If I skip the
validation the output looks OK, but obviously I need to find out what is going
on.  Looking for some suggestions to debug.  Also, currently clist does NOT
work properly -- so run with small dpi or set the buffers appropriatly.  There
is certain information that must be passed via the clist writer to the reader
when an Smask is pushed.  I will get that defined shortly.  Also, the case with
ICC color spaces is not currently implemented. Finally, I still need to test
separation devices.  The patch includes the changes Ray has done to the PDF PS
interpreter files (pdf_draw.ps and pdf_ops.ps).  The main focus of this patch
over the one it is replacing is that the Smask buffer colors were getting
mapped to the device color space defined by the process color model.  This is
not correct, as they should be mapped to the transparency color space for the
group.	To achieve this, several of the PDF14 device color mapping and blending
procs are altered based upon the current blending space when an Smask group is
pushed.  When the Smask group is popped, the device procs are returned to their
original values and the Smask is rendered into a single band buffer.  There is
a larger number of buffer output debugging code that has been added.  It will
dump raw image buffers that are easily viewed in Photoshop.  To activate set
#define RAW_DUMP 1 in gxblend.h.  The buffers are dumped with dimensions in the
file name that are num_cols by num_rows by num_channels.  A global prefix
counter is used to keep track of the compositing groupings and order of the
buffers.
Comment 14 Michael Vrhel 2008-10-02 22:16:26 UTC
Created attachment 4464 [details]
SmaskPatch2.diff

This patch replaces the previous one.  I found and fixed the memory issue and
also did the change recommended by Igor to make sure we only fill in the
defined rect region.  Still need to take care of a some CIE issues in
particular, ICC color spaces.  Also, I need to test separation output and get
the clist parameter requirements defined.
Comment 15 Michael Vrhel 2009-02-16 20:22:35 UTC
This renders properly with the soft mask branch
Comment 16 Michael Vrhel 2009-04-28 14:40:47 UTC
With the merge of the soft mask branch, this and the duplicate bugs are fixed.