Remove Agg from names of CFX_AggClipRgn and CFX_AggImageRenderer. [pdfium : main]

[Tom Sepez (Gerrit)]

Attention needed from Lei Zhang

Tom Sepez added 2 comments

File core/fxge/cfx_renderdevice.cpp

Line 1054, Patchset 3 (Latest):bool CFX_RenderDevice::ContinueDIBits(CFX_ImageRenderer* handle,

Tom Sepez . resolved

This was one of the layering violations behind this CL.

File core/fxge/dib/cfx_dibbase.h

Line 115, Patchset 3 (Latest): const CFX_ClipRgn* pClipRgn) const;

Tom Sepez . resolved

Layering here, too. You get the idea.

Open in Gerrit

Related details

Attention is currently required from:

• Lei Zhang

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I663dee295560808c048577fd2d02c14a251acb7e

Gerrit-Change-Number: 143412

Gerrit-PatchSet: 3

Gerrit-Owner: Tom Sepez <tse...@chromium.org>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: Pdfium LUCI CQ <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: pdfium-...@googlegroups.com

Gerrit-Attention: Lei Zhang <the...@chromium.org>

Gerrit-Comment-Date: Fri, 20 Feb 2026 18:44:41 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Lei Zhang (Gerrit)]

Attention needed from Tom Sepez

Lei Zhang added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Lei Zhang . resolved

But Code Search shows these 2 classes are only instatiated inside cfx_agg_devicedriver.cpp. So unless Code Search is impcomplete, I would say the current state is the rest of the code-base puts up with these AGG-isms.

Open in Gerrit

Related details

Attention is currently required from:

• Tom Sepez

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I663dee295560808c048577fd2d02c14a251acb7e

Gerrit-Change-Number: 143412

Gerrit-PatchSet: 3

Gerrit-Owner: Tom Sepez <tse...@chromium.org>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: Pdfium LUCI CQ <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: pdfium-...@googlegroups.com

Gerrit-Attention: Tom Sepez <tse...@chromium.org>

Gerrit-Comment-Date: Fri, 20 Feb 2026 18:49:19 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Lei Zhang (Gerrit)]

Attention needed from Tom Sepez

Lei Zhang added 1 comment

File core/fxge/dib/cfx_dibbase.h

Line 115, Patchset 3 (Latest): const CFX_ClipRgn* pClipRgn) const;

Tom Sepez . resolved

Layering here, too. You get the idea.

Lei Zhang

Using this as an example, if one looks at the callers and callers' callers, only the AGG .cpp files pass in non-null arguments. Other files either pass in nullptr, or pass in a value that was passed to them.

Open in Gerrit

Related details

Attention is currently required from:

• Tom Sepez

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I663dee295560808c048577fd2d02c14a251acb7e

Gerrit-Change-Number: 143412

Gerrit-PatchSet: 3

Gerrit-Owner: Tom Sepez <tse...@chromium.org>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: Pdfium LUCI CQ <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: pdfium-...@googlegroups.com

Gerrit-Attention: Tom Sepez <tse...@chromium.org>

Gerrit-Comment-Date: Fri, 20 Feb 2026 18:53:23 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Tom Sepez <tse...@chromium.org>

[Tom Sepez (Gerrit)]

Tom Sepez added 4 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Tom Sepez . resolved

Yeah, this might be made to work, but searching for agg really shouldn't give any hits outside of agg ...

File core/fxge/renderdevicedriver_iface.h

Line 53, Patchset 3 (Latest): std::unique_ptr<CFX_ImageRenderer> agg_image_renderer);

Tom Sepez . unresolved

This kinda feels like a layering violation as well.

File testing/fuzzers/pdf_scanlinecompositor_fuzzer.cc

Line 80, Patchset 3 (Latest): std::unique_ptr<CFX_ClipRgn> clip_rgn;

Tom Sepez . unresolved

this would be a layering violation as well.

File xfa/fxfa/cxfa_imagerenderer.h

Line 38, Patchset 3 (Latest): std::unique_ptr<CFX_ImageRenderer> device_handle_;

Tom Sepez . unresolved

This feels like a layering violation as well.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Owners
• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I663dee295560808c048577fd2d02c14a251acb7e

Gerrit-Change-Number: 143412

Gerrit-PatchSet: 3

Gerrit-Owner: Tom Sepez <tse...@chromium.org>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: Pdfium LUCI CQ <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: pdfium-...@googlegroups.com

Gerrit-Comment-Date: Fri, 20 Feb 2026 19:25:48 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Tom Sepez (Gerrit)]

Tom Sepez abandoned this change.

View Change

Abandoned Ok, I have another Idea you might like better.

Tom Sepez abandoned this change

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: abandon