diff options
author | Ken Sharp <ken.sharp@artifex.com> | 2023-02-28 15:36:56 +0000 |
---|---|---|
committer | Ken Sharp <ken.sharp@artifex.com> | 2023-02-28 15:39:09 +0000 |
commit | 6894a2826210baf24f9ccd024bbca211b17d1f9b (patch) | |
tree | eeb4b964e285b4173f0170a6cee990c737ed6058 /base | |
parent | 1a722c886c4d3fbd9931b627a17270aca9701606 (diff) | |
download | ghostpdl-6894a2826210baf24f9ccd024bbca211b17d1f9b.tar.gz |
GhostPDF + pdfwrite - emit an Alternate for some ICCBased spaces.
Bug #705865 "PDF Writer is dropping the /Alternate color space for ICC based color profile"
This commit starts off with the bug referenced above. Prior to this
commit the PDF interpreter never set the /Alternate space for an
ICCBased colour space. For rendering there is no need since Ghostscript
will always use the ICC profile, or will set the /Alternate instead
(if there is an error in the ICC profile) or will use the number of
components (/N) to set a Device space if there is a fault in the ICC
profile and no /Alternate.
However, this means pdfwrite never sees an Alternate space to write out
for an ICCBased space. This should not be a problem since the
/Alternate is optional for an ICCBased space and indeed the PDF 2.0
specification states "PDF readers should not use this colour space".
The file for the bug has a /ICCBased space where the /Alternate is
an Lab space. Obviously any PDF consumer should use the ICCBased space
but it seems that Chrome, Firefox, possibly other browsers cannot handle
ICCBased colour and so drop back to the Alternate. Surprisingly they
can handle Lab and get the expected colour. Obviously if we drop the
/Alternate then these consumers cannot use Lab and have a last ditch
fallback to RGB (based on the number of components, and that *is* in the
spec). But RGB != Lab and so the colours are incorrect.
Ideally we would simply use the existing colour space code and attach
the alternate space to the ICCBased space's 'base_space' member. That
would write everything perfectly well. But we can't do that because
when we are called from Ghostscript the ICC profile cache is in GC'ed
memory. So we have to create the ICCBased colour space in GC'ed memory
too. We have special hackery in the PDF interpreter colour code to do
exactly that.
Colour spaces call back to the PDF interpreter when freed (with more
hackery for ICCBased spaces), but if we create colour spaces in non-GC
(PDF interpreter) memory and attach them to the ICCBased space in GC'ed
memory then they can outlive the PDF interpreter, leading to crashes.
I did start down the road of making all colour spaces in GC-ed memory,
but that rapidly spiralled out of control because names needed to be
allocated in GC'ed memory, and functions and well, all kinds of things.
Without that we got crashes, and it quickly became clear the only real
way to make this work would be to allocate everything in GC'ed memory
which we really don't want to do.
So instead I added a new enumerated type member to the colour space, in
that member, if the current colour space is ICCBased, we store the type
of Alternate that was supplied (if any). We only support DeviceGray,
DeviceRGB, DeviceCMYK, CalGray, CalRGB and Lab. I also added the
relevant parameters to the 'params' union of the colour space.
In the PDF interpreter; add code to spot the afore-mentioned Alternate
spaces if present, and if we haven't been forced to fall back to using
the Alternate (or /N) because the ICC profile is broken. When we spot
one of those spaces, set the colour space ICC_Alternate_space member
appropriately and for the paramaterised spaces gather the parameter
values and store them.
In the pdfwrite device; if we are writing out an ICCBased space, and
it's ICC_Alternate_space member is not gs_ICC_Alternate_none, create
a ColorSpace resource and call the relevant space-specific code to
create a colour space array with a name and dictionary containing the
required parameters. Attach the resulting object to the ICCBased
colour space by inserting it into the array with a /Alternate key.
This also meant I needed to alter the parameters passed internally to
pdf_make_iccbased so that we pass in the original colour space instead
of the alternate space (which is always NULL currently).
There are also a couple of fixes; when finalising a colour space check
that the colour space is a DeviceN space before checking if the device_n
structure in the params union has a non-zero devn_process_space. The new
members in the union meant we could get here and think we needed to
free the devn_process_space, causing a crash.
In the PDF interpreter; there's a little clause in the PDF specification
which mentions a CalCMYK space. Apparently this was never properly
specified and so should be treated as DeviceCMYK. The PDF interpreter
now does so.
Finally another obsrvation; the initial code wrote the /Alternate space
as a named colour space, eg:
19 0 obj
<</N 3
/Alternate /R18 /Length 1972>>stream
....
Where R18 is defined in the Page's ColorSpace Resources as a named
resource:
<</R18
18 0 R/R17
17 0 R/R20
20 0 R/R22
22 0 R>>
endobj
But this does not work with Chrome (I didn't test Firefox). For this to
work with Chrome we have to reference the object directly, which should
not be required IMO. I believe this to be (another) bug in Chrome's PDF
handling.
Diffstat (limited to 'base')
-rw-r--r-- | base/gscspace.c | 11 | ||||
-rw-r--r-- | base/gscspace.h | 40 |
2 files changed, 45 insertions, 6 deletions
diff --git a/base/gscspace.c b/base/gscspace.c index d662bc025..9a707a0c0 100644 --- a/base/gscspace.c +++ b/base/gscspace.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2001-2021 Artifex Software, Inc. +/* Copyright (C) 2001-2023 Artifex Software, Inc. All Rights Reserved. This software is provided AS-IS with no warranty, either express or @@ -109,9 +109,11 @@ gs_cspace_final(const gs_memory_t *cmem, void *vptr) if_debug2m('c', cmem, "[c]cspace final "PRI_INTPTR" %d\n", (intptr_t)pcs, (int)pcs->id); rc_decrement_only_cs(pcs->base_space, "gs_cspace_final"); pcs->base_space = NULL; - if (pcs->params.device_n.devn_process_space != NULL) { - rc_decrement_only_cs(pcs->params.device_n.devn_process_space, "gs_cspace_final"); - pcs->params.device_n.devn_process_space = NULL; + if (gs_color_space_get_index(pcs) == gs_color_space_index_DeviceN) { + if (pcs->params.device_n.devn_process_space != NULL) { + rc_decrement_only_cs(pcs->params.device_n.devn_process_space, "gs_cspace_final"); + pcs->params.device_n.devn_process_space = NULL; + } } /* No need to decrement the ICC profile data. It is handled by the finalize of the ICC space which is called above using @@ -135,6 +137,7 @@ gs_cspace_alloc_with_id(gs_memory_t *mem, ulong id, pcs->interpreter_data = NULL; pcs->interpreter_free_cspace_proc = NULL; pcs->cmm_icc_profile_data = NULL; + pcs->ICC_Alternate_space = gs_ICC_Alternate_None; pcs->icc_equivalent = NULL; pcs->params.device_n.devn_process_space = NULL; return pcs; diff --git a/base/gscspace.h b/base/gscspace.h index 91bb7c784..9e9e46263 100644 --- a/base/gscspace.h +++ b/base/gscspace.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2001-2022 Artifex Software, Inc. +/* Copyright (C) 2001-2023 Artifex Software, Inc. All Rights Reserved. This software is provided AS-IS with no warranty, either express or @@ -285,6 +285,25 @@ typedef struct gs_pattern_params_s { bool has_base_space; /* {csrc} can't we just NULL-check the base_space? */ } gs_pattern_params; +typedef struct gs_calgray_params_s { + float WhitePoint[3]; + float BlackPoint[3]; + float Gamma; +} gs_calgray_params; + +typedef struct gs_calrgb_params_s { + float WhitePoint[3]; + float BlackPoint[3]; + float Gamma[3]; + float Matrix[9]; +} gs_calrgb_params; + +typedef struct gs_lab_params_s { + float WhitePoint[3]; + float BlackPoint[3]; + float Range[4]; +} gs_lab_params; + /* id's 1 through 4 are reserved for static colorspaces; thus, dynamically assigned id's must begin at 5. */ #define cs_DeviceGray_id 1 @@ -293,6 +312,16 @@ typedef struct gs_pattern_params_s { typedef void (*gs_cspace_free_proc_t) (gs_memory_t * mem, void *pcs); +typedef enum { + gs_ICC_Alternate_None, + gs_ICC_Alternate_DeviceGray, + gs_ICC_Alternate_DeviceRGB, + gs_ICC_Alternate_DeviceCMYK, + gs_ICC_Alternate_CalGray, + gs_ICC_Alternate_CalRGB, + gs_ICC_Alternate_Lab, +} gs_ICC_Alternate_space; + /* * The colorspace object. For pattern and indexed colorspaces, the * base_space refers to the underlying colorspace. For separation, @@ -306,6 +335,7 @@ struct gs_color_space_s { gs_id id; gs_color_space *base_space; gs_color_space *icc_equivalent; + gs_ICC_Alternate_space ICC_Alternate_space; client_color_space_data_t *pclient_color_space_data; void *interpreter_data; gs_cspace_free_proc_t interpreter_free_cspace_proc; @@ -320,7 +350,13 @@ struct gs_color_space_s { gs_device_n_params device_n; gs_indexed_params indexed; gs_pattern_params pattern; - + /* These are only used for the Alternate space of an ICCBased + * space, for the benefit of pdfwrite. For rendering, these + * spaces are converted into ICCBased spaces. + */ + gs_calgray_params calgray; + gs_calrgb_params calrgb; + gs_lab_params lab; } params; }; |