Fix PDF saving when multiple graphics states are in effect
To avoid saving unnecessary data, CL [1] added the ability to keep track
of which ExtGState resources were in use. This does not work in all
cases, like the newly created multiple_graphics_states.pdf being added
in this CL. To fix this case where multiple graphics states are in
effect at the same time, change the code that keeps track of ExtGState
resources to store a vector of resource names, instead of just the most
recently used one.
[1] https://pdfium-review.googlesource.com/105614
Bug: chromium:1451953
Change-Id: I673fe9f13efe90c50dbfa2511ab57092a2f987e3
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/108471
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index c79de4c..7654bc1 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -81,10 +81,9 @@
break;
}
}
- const ByteString& graphics_resource_name =
- page_object->GetGraphicsResourceName();
- if (!graphics_resource_name.IsEmpty()) {
- seen_resources["ExtGState"].insert(graphics_resource_name);
+ for (const auto& name : page_object->GetGraphicsResourceNames()) {
+ CHECK(!name.IsEmpty());
+ seen_resources["ExtGState"].insert(name);
}
}
@@ -612,7 +611,7 @@
}
m_pDocument->AddIndirectObject(gsDict);
name = RealizeResource(std::move(gsDict), "ExtGState");
- pPageObj->SetGraphicsResourceName(name);
+ pPageObj->SetGraphicsResourceNames({name});
m_pObjHolder->GraphicsMapInsert(graphD, name);
}
*buf << "/" << PDF_NameEncode(name) << " gs ";
diff --git a/core/fpdfapi/page/cpdf_allstates.cpp b/core/fpdfapi/page/cpdf_allstates.cpp
index a4330db..3002eed 100644
--- a/core/fpdfapi/page/cpdf_allstates.cpp
+++ b/core/fpdfapi/page/cpdf_allstates.cpp
@@ -26,7 +26,7 @@
void CPDF_AllStates::Copy(const CPDF_AllStates& src) {
CopyStates(src);
- m_GraphicsResourceName = src.m_GraphicsResourceName;
+ m_GraphicsResourceNames = src.m_GraphicsResourceNames;
m_TextMatrix = src.m_TextMatrix;
m_ParentMatrix = src.m_ParentMatrix;
m_CTM = src.m_CTM;
diff --git a/core/fpdfapi/page/cpdf_allstates.h b/core/fpdfapi/page/cpdf_allstates.h
index 63eb527..3d4d9a3 100644
--- a/core/fpdfapi/page/cpdf_allstates.h
+++ b/core/fpdfapi/page/cpdf_allstates.h
@@ -7,6 +7,8 @@
#ifndef CORE_FPDFAPI_PAGE_CPDF_ALLSTATES_H_
#define CORE_FPDFAPI_PAGE_CPDF_ALLSTATES_H_
+#include <vector>
+
#include "core/fpdfapi/page/cpdf_graphicstates.h"
#include "core/fxcrt/bytestring.h"
#include "core/fxcrt/fx_coordinates.h"
@@ -25,7 +27,7 @@
CPDF_StreamContentParser* pParser);
void SetLineDash(const CPDF_Array* pArray, float phase, float scale);
- ByteString m_GraphicsResourceName;
+ std::vector<ByteString> m_GraphicsResourceNames;
CFX_Matrix m_TextMatrix;
CFX_Matrix m_CTM;
CFX_Matrix m_ParentMatrix;
diff --git a/core/fpdfapi/page/cpdf_pageobject.cpp b/core/fpdfapi/page/cpdf_pageobject.cpp
index 8636af6..a7b2156 100644
--- a/core/fpdfapi/page/cpdf_pageobject.cpp
+++ b/core/fpdfapi/page/cpdf_pageobject.cpp
@@ -6,6 +6,8 @@
#include "core/fpdfapi/page/cpdf_pageobject.h"
+#include <utility>
+
#include "core/fxcrt/fx_coordinates.h"
CPDF_PageObject::CPDF_PageObject(int32_t content_stream)
@@ -73,6 +75,11 @@
return nullptr;
}
+void CPDF_PageObject::SetGraphicsResourceNames(
+ std::vector<ByteString> resource_names) {
+ m_GraphicsResourceNames = std::move(resource_names);
+}
+
void CPDF_PageObject::CopyData(const CPDF_PageObject* pSrc) {
CopyStates(*pSrc);
m_Rect = pSrc->m_Rect;
diff --git a/core/fpdfapi/page/cpdf_pageobject.h b/core/fpdfapi/page/cpdf_pageobject.h
index 7d9d015..ccdbbaf 100644
--- a/core/fpdfapi/page/cpdf_pageobject.h
+++ b/core/fpdfapi/page/cpdf_pageobject.h
@@ -9,6 +9,8 @@
#include <stdint.h>
+#include <vector>
+
#include "core/fpdfapi/page/cpdf_contentmarks.h"
#include "core/fpdfapi/page/cpdf_graphicstates.h"
#include "core/fxcrt/bytestring.h"
@@ -94,12 +96,10 @@
m_ResourceName = resource_name;
}
- const ByteString& GetGraphicsResourceName() const {
- return m_GraphicsResourceName;
+ const std::vector<ByteString>& GetGraphicsResourceNames() const {
+ return m_GraphicsResourceNames;
}
- void SetGraphicsResourceName(const ByteString& resource_name) {
- m_GraphicsResourceName = resource_name;
- }
+ void SetGraphicsResourceNames(std::vector<ByteString> resource_names);
protected:
void CopyData(const CPDF_PageObject* pSrcObject);
@@ -110,8 +110,11 @@
CPDF_ContentMarks m_ContentMarks;
bool m_bDirty = false;
int32_t m_ContentStream;
- ByteString m_ResourceName; // The resource name for this object.
- ByteString m_GraphicsResourceName; // Like `m_ResourceName` but for graphics.
+ // The resource name for this object.
+ ByteString m_ResourceName;
+ // Like `m_ResourceName` but for graphics. Though unlike the resource name,
+ // multiple graphics states can apply at once.
+ std::vector<ByteString> m_GraphicsResourceNames;
};
#endif // CORE_FPDFAPI_PAGE_CPDF_PAGEOBJECT_H_
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 03acebb..442b2aa 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -429,7 +429,7 @@
if (bText) {
pObj->m_TextState = m_pCurStates->m_TextState;
}
- pObj->SetGraphicsResourceName(m_pCurStates->m_GraphicsResourceName);
+ pObj->SetGraphicsResourceNames(m_pCurStates->m_GraphicsResourceNames);
}
// static
@@ -790,7 +790,7 @@
auto pFormObj = std::make_unique<CPDF_FormObject>(GetCurrentStreamIndex(),
std::move(form), matrix);
pFormObj->SetResourceName(name);
- pFormObj->SetGraphicsResourceName(m_pCurStates->m_GraphicsResourceName);
+ pFormObj->SetGraphicsResourceNames(m_pCurStates->m_GraphicsResourceNames);
if (!m_pObjectHolder->BackgroundAlphaNeeded() &&
pFormObj->form()->BackgroundAlphaNeeded()) {
m_pObjectHolder->SetBackgroundAlphaNeeded(true);
@@ -915,7 +915,8 @@
if (!pGS)
return;
- m_pCurStates->m_GraphicsResourceName = name;
+ CHECK(!name.IsEmpty());
+ m_pCurStates->m_GraphicsResourceNames.push_back(std::move(name));
m_pCurStates->ProcessExtGS(pGS.Get(), this);
}
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 670892a..7b973ea 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -4647,3 +4647,33 @@
UnloadPage(page);
}
+
+TEST_F(FPDFEditEmbedderTest, MultipleGraphicsStates) {
+ ASSERT_TRUE(OpenDocument("multiple_graphics_states.pdf"));
+ FPDF_PAGE page = LoadPage(0);
+ ASSERT_TRUE(page);
+
+ {
+ ScopedFPDFPageObject path(FPDFPageObj_CreateNewPath(400, 100));
+ EXPECT_TRUE(FPDFPageObj_SetFillColor(path.get(), 255, 0, 0, 255));
+ EXPECT_TRUE(FPDFPath_SetDrawMode(path.get(), FPDF_FILLMODE_ALTERNATE, 0));
+ EXPECT_TRUE(FPDFPath_MoveTo(path.get(), 100, 100));
+ EXPECT_TRUE(FPDFPath_LineTo(path.get(), 100, 125));
+ EXPECT_TRUE(FPDFPath_Close(path.get()));
+
+ FPDFPage_InsertObject(page, path.release());
+ EXPECT_TRUE(FPDFPage_GenerateContent(page));
+ }
+
+ const char* checksum = CFX_DefaultRenderDevice::SkiaIsDefaultRenderer()
+ ? "7ebec75d95c64b522999a710de76c52c"
+ : "f4b36616a7fea81a4f06cc7b01a55ac1";
+
+ ScopedFPDFBitmap bitmap = RenderPage(page);
+ CompareBitmap(bitmap.get(), 200, 300, checksum);
+
+ ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+ VerifySavedDocument(200, 300, checksum);
+
+ UnloadPage(page);
+}
diff --git a/testing/resources/multiple_graphics_states.in b/testing/resources/multiple_graphics_states.in
new file mode 100644
index 0000000..882495f
--- /dev/null
+++ b/testing/resources/multiple_graphics_states.in
@@ -0,0 +1,58 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /MediaBox [0 0 200 300]
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Contents 4 0 R
+ /Resources <<
+ /ExtGState <<
+ /GS1 5 0 R
+ /GS2 6 0 R
+ >>
+ >>
+>>
+endobj
+{{object 4 0}} <<
+ {{streamlen}}
+>>
+stream
+q
+0 0 0 rg
+0 290 10 10 re B*
+10 150 50 30 re B*
+0 0 1 rg
+190 290 10 10 re B*
+70 232 50 30 re B*
+0 1 0 rg
+190 0 10 10 re B*
+130 150 50 30 re B*
+/GS1 gs /GS2 gs
+1 0 0 rg
+0 0 10 10 re B*
+70 67 50 30 re B*
+Q
+endstream
+endobj
+{{object 5 0}} <<
+ /ca 0.25
+>>
+endobj
+{{object 6 0}} <<
+ /LW 4
+>>
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/multiple_graphics_states.pdf b/testing/resources/multiple_graphics_states.pdf
new file mode 100644
index 0000000..d9a8d4a
--- /dev/null
+++ b/testing/resources/multiple_graphics_states.pdf
@@ -0,0 +1,71 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+>>
+endobj
+2 0 obj <<
+ /Type /Pages
+ /MediaBox [0 0 200 300]
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /Contents 4 0 R
+ /Resources <<
+ /ExtGState <<
+ /GS1 5 0 R
+ /GS2 6 0 R
+ >>
+ >>
+>>
+endobj
+4 0 obj <<
+ /Length 204
+>>
+stream
+q
+0 0 0 rg
+0 290 10 10 re B*
+10 150 50 30 re B*
+0 0 1 rg
+190 290 10 10 re B*
+70 232 50 30 re B*
+0 1 0 rg
+190 0 10 10 re B*
+130 150 50 30 re B*
+/GS1 gs /GS2 gs
+1 0 0 rg
+0 0 10 10 re B*
+70 67 50 30 re B*
+Q
+endstream
+endobj
+5 0 obj <<
+ /ca 0.25
+>>
+endobj
+6 0 obj <<
+ /LW 4
+>>
+endobj
+xref
+0 7
+0000000000 65535 f
+0000000015 00000 n
+0000000068 00000 n
+0000000157 00000 n
+0000000306 00000 n
+0000000562 00000 n
+0000000594 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 7
+>>
+startxref
+623
+%%EOF
