From 72439636682293fa0e4ce210ba1c3602b541836b Mon Sep 17 00:00:00 2001 From: OnionsYu Date: Tue, 28 Jul 2026 19:25:01 +0800 Subject: [PATCH 1/6] Fix HTML radial-gradient circle extent sizing and transparent stops fading through black on export. --- src/pagx/html/importer/HTMLValueParser.cpp | 151 +++++++++++++++++---- test/src/PAGXHTMLImporterTest.cpp | 41 ++++++ 2 files changed, 165 insertions(+), 27 deletions(-) diff --git a/src/pagx/html/importer/HTMLValueParser.cpp b/src/pagx/html/importer/HTMLValueParser.cpp index bad3bec673..767f4391e0 100644 --- a/src/pagx/html/importer/HTMLValueParser.cpp +++ b/src/pagx/html/importer/HTMLValueParser.cpp @@ -627,6 +627,43 @@ Color SampleRepeatingPeriod(const HTMLValueParser::GradientStops& stops, float f return stops.back().second; } +// CSS radial extent keywords control how far the ending shape reaches; they carry no scalar radius +// in the token itself (the radius is derived from the center and box). Returns true for any of the +// four keywords so the caller can compute the corresponding px radius from the center position. +bool IsRadialExtentKeyword(const std::string& token) { + return token == "closest-side" || token == "closest-corner" || token == "farthest-side" || + token == "farthest-corner"; +} + +// Computes the px radius of a CSS `circle` ending shape for the given extent keyword, measured from +// a center at (cxPx, cyPx) within a (0,0)-(boxWidth,boxHeight) box. An empty/unknown keyword +// defaults to `farthest-corner`, matching CSS when the size is omitted. `closest-corner` / +// `farthest-corner` are the Euclidean distances to the nearest / farthest box corner; +// `closest-side` / `farthest-side` are the min / max of the perpendicular distances to the four +// edges. +float CircleExtentRadiusPx(const std::string& keyword, float cxPx, float cyPx, float boxWidth, + float boxHeight) { + float left = cxPx; + float right = boxWidth - cxPx; + float top = cyPx; + float bottom = boxHeight - cyPx; + float dx = std::max(left, right); + float dy = std::max(top, bottom); + if (keyword == "closest-side") { + return std::min(std::min(left, right), std::min(top, bottom)); + } + if (keyword == "farthest-side") { + return std::max(dx, dy); + } + if (keyword == "closest-corner") { + float nx = std::min(left, right); + float ny = std::min(top, bottom); + return std::sqrt(nx * nx + ny * ny); + } + // farthest-corner (also the default when the size is omitted). + return std::sqrt(dx * dx + dy * dy); +} + } // namespace ColorSource* HTMLValueParser::parseRepeatingLinearGradientPattern(const std::string& value, @@ -924,24 +961,10 @@ void HTMLValueParser::parseRadialDescriptor(const std::string& descriptor, float } } - // Radius: the exporter writes `rx = radius * boxWidth` (and an ellipse's `ry` is implied by the - // box height under PAGX's single-radius + fitsToGeometry model), so a length token divided by - // boxWidth recovers the normalised radius. A bare `%` is already box-relative. Track whether - // the radius came from an explicit px length so a circle on a non-square box can later switch to - // the fitsToGeometry=false pixel model (see below). - bool radiusFromPxLength = false; - if (!sizeTokens.empty() && boxWidth > 0) { - float radius = resolveRadialLength(sizeTokens[0], boxWidth); - if (!std::isnan(radius)) { - grad->radius = radius; - radiusFromPxLength = !sizeTokens[0].empty() && sizeTokens[0].back() != '%'; - } else { - // Extent keywords (closest-side / farthest-corner / ...) have no scalar PAGX radius; keep - // the box-filling default and surface a diagnostic instead of silently mis-sizing. - _diagnostics.warn("html: radial-gradient size '" + sizeTokens[0] + - "' not supported; using box-filling radius"); - } - } + // A circle keeps one uniform radius; an ellipse (explicit, or the shapeless default) stretches + // per axis. Detected up front because extent-keyword and omitted sizes resolve the radius from + // the center, which the position pass below establishes first. + bool isCircle = explicitCircle || (!explicitEllipse && sizeTokens.size() == 1); // Position: `at `. Axis-locked keywords (left/right -> x, top/bottom -> y) are assigned // first so author order is irrelevant (`at top left` == `at left top`); the remaining `center` @@ -972,15 +995,58 @@ void HTMLValueParser::parseRadialDescriptor(const std::string& descriptor, float if (!std::isnan(cx)) grad->center.x = cx; if (!std::isnan(cy)) grad->center.y = cy; - // A CSS `circle px` keeps a single uniform radius regardless of box aspect ratio. PAGX's - // default fitsToGeometry=true model stretches the normalised radius by box width and height - // independently, so on a non-square box it would render the circle as an ellipse. Switch such a - // circle to the fitsToGeometry=false pixel model (center/radius in the geometry's local px - // space, where the box spans (0,0)-(boxWidth,boxHeight)) so the radius stays isotropic. Square - // boxes, ellipses, and percentage/extent sizes keep the compact normalised representation. - bool isCircle = explicitCircle || (!explicitEllipse && sizeTokens.size() == 1); - if (isCircle && radiusFromPxLength && boxWidth > 0 && boxHeight > 0 && - std::abs(boxWidth - boxHeight) > 0.01f) { + // Radius: a length token divided by boxWidth recovers the normalised radius (a bare `%` is + // already box-relative); track whether it came from an explicit px length so a circle on a + // non-square box can later switch to the fitsToGeometry=false pixel model. An extent keyword + // (or, for a circle, an omitted size — CSS defaults it to farthest-corner) has no scalar radius + // in the token, so a circle derives the px radius from its center and the box; `circleExtentPx` + // then routes it through the pixel model below since the value is already in px. + bool radiusFromPxLength = false; + bool circleExtentPx = false; + if (!sizeTokens.empty() && boxWidth > 0) { + float radius = resolveRadialLength(sizeTokens[0], boxWidth); + if (!std::isnan(radius)) { + grad->radius = radius; + radiusFromPxLength = !sizeTokens[0].empty() && sizeTokens[0].back() != '%'; + } else if (IsRadialExtentKeyword(sizeTokens[0])) { + // Only an explicit `circle` maps cleanly to PAGX's single radius. An implicit shape with an + // extent keyword (or an explicit ellipse) is an ellipse in CSS and needs per-axis radii the + // model can't represent, so keep the box-filling default and surface a diagnostic. + if (explicitCircle && boxHeight > 0) { + grad->radius = CircleExtentRadiusPx(sizeTokens[0], grad->center.x * boxWidth, + grad->center.y * boxHeight, boxWidth, boxHeight); + circleExtentPx = true; + } else { + _diagnostics.warn("html: radial-gradient size '" + sizeTokens[0] + + "' not supported; using box-filling radius"); + } + } else { + _diagnostics.warn("html: radial-gradient size '" + sizeTokens[0] + + "' not supported; using box-filling radius"); + } + } else if (sizeTokens.empty() && explicitCircle && boxWidth > 0 && boxHeight > 0) { + // A `circle` with no size defaults to farthest-corner in CSS. + grad->radius = CircleExtentRadiusPx("", grad->center.x * boxWidth, grad->center.y * boxHeight, + boxWidth, boxHeight); + circleExtentPx = true; + } + + // Keep a circle's single radius isotropic. The default fitsToGeometry=true model scales the + // normalised radius by box width and height independently, so on a non-square box it would render + // a circle as an ellipse; such circles switch to the fitsToGeometry=false pixel model (center / + // radius in the geometry's local px space, where the box spans (0,0)-(boxWidth,boxHeight)). On a + // square box the normalised model is already isotropic, so keep the compact representation: + // extent/omitted sizes carry a px radius that is normalised back by boxWidth, while an explicit + // px length was already normalised above. Ellipses and percentage sizes stay normalised too. + bool nonSquare = boxWidth > 0 && boxHeight > 0 && std::abs(boxWidth - boxHeight) > 0.01f; + if (circleExtentPx) { + if (nonSquare) { + grad->center = {grad->center.x * boxWidth, grad->center.y * boxHeight}; + grad->fitsToGeometry = false; + } else { + grad->radius = grad->radius / boxWidth; + } + } else if (isCircle && radiusFromPxLength && nonSquare) { grad->center = {grad->center.x * boxWidth, grad->center.y * boxHeight}; grad->radius = grad->radius * boxWidth; grad->fitsToGeometry = false; @@ -1083,6 +1149,37 @@ bool HTMLValueParser::finaliseGradientStops(GradientStops& stops) { float steps = static_cast(next - (i - 1)); stops[i].first = prevOffset + (nextOffset - prevOffset) / steps; } + + // CSS interpolates gradient stops in premultiplied-alpha space, so a `transparent` (or any + // alpha=0) stop contributes only its neighbour's colour as the alpha fades — e.g. a + // `rgba(220,210,255,0.4) -> transparent` ramp stays purple while vanishing. The renderer + // interpolates unpremultiplied, where a keyword `transparent` carries black RGB and would drag + // the ramp toward grey/black. Rewrite each fully transparent stop's RGB to that of its nearest + // opaque neighbour (alpha kept at 0) so the unpremultiplied interpolation matches CSS. A stop + // between two opaque colours prefers the earlier neighbour, matching the premultiplied midpoint. + for (size_t i = 0; i < stops.size(); ++i) { + if (stops[i].second.alpha > 0.0f) continue; + size_t donor = stops.size(); + for (size_t back = i; back-- > 0;) { + if (stops[back].second.alpha > 0.0f) { + donor = back; + break; + } + } + if (donor == stops.size()) { + for (size_t fwd = i + 1; fwd < stops.size(); ++fwd) { + if (stops[fwd].second.alpha > 0.0f) { + donor = fwd; + break; + } + } + } + if (donor != stops.size()) { + stops[i].second.red = stops[donor].second.red; + stops[i].second.green = stops[donor].second.green; + stops[i].second.blue = stops[donor].second.blue; + } + } return true; } diff --git a/test/src/PAGXHTMLImporterTest.cpp b/test/src/PAGXHTMLImporterTest.cpp index 3cb518f0a1..191ae520b0 100644 --- a/test/src/PAGXHTMLImporterTest.cpp +++ b/test/src/PAGXHTMLImporterTest.cpp @@ -1143,6 +1143,47 @@ PAG_TEST(PAGXHTMLImporterTest, RadialGradientEllipseOnNonSquareBoxKeepsNormalise EXPECT_TRUE(rg->fitsToGeometry); } +PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleOmittedSizeUsesFarthestCorner) { + auto doc = ParseFromString(R"HTML( + +
+ + )HTML"); + ASSERT_NE(doc, nullptr); + auto* div = doc->layers.front()->children.front(); + auto* fill = FindElementOfType(div); + ASSERT_NE(fill, nullptr); + auto* rg = As(fill->color); + ASSERT_NE(rg, nullptr); + // A `circle` with no size defaults to farthest-corner: center (100,50) on a 400x200 box reaches + // the farthest corner (400,200) at sqrt(300^2 + 150^2) ~= 335.4px. The pixel model keeps the + // radius isotropic, so center/radius are stored in px with fitsToGeometry disabled. + EXPECT_FALSE(rg->fitsToGeometry); + EXPECT_TRUE(NearlyEqual(rg->center.x, 100.0f, 0.5f)); + EXPECT_TRUE(NearlyEqual(rg->center.y, 50.0f, 0.5f)); + EXPECT_TRUE(NearlyEqual(rg->radius, 335.41f, 0.5f)); +} + +PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleClosestSideExtent) { + auto doc = ParseFromString(R"HTML( + +
+ + )HTML"); + ASSERT_NE(doc, nullptr); + auto* div = doc->layers.front()->children.front(); + auto* fill = FindElementOfType(div); + ASSERT_NE(fill, nullptr); + auto* rg = As(fill->color); + ASSERT_NE(rg, nullptr); + // closest-side: center (100,100) on a 400x200 box; nearest edge distances are left=100, + // right=300, top=100, bottom=100, so the radius is 100px in the isotropic pixel model. + EXPECT_FALSE(rg->fitsToGeometry); + EXPECT_TRUE(NearlyEqual(rg->center.x, 100.0f, 0.5f)); + EXPECT_TRUE(NearlyEqual(rg->center.y, 100.0f, 0.5f)); + EXPECT_TRUE(NearlyEqual(rg->radius, 100.0f, 0.5f)); +} + PAG_TEST(PAGXHTMLImporterTest, ConicGradientAngleOffset) { auto doc = ParseFromString(R"HTML( From fbd7d95de5b61a2c836b3df281bcd6acb47c7dcd Mon Sep 17 00:00:00 2001 From: OnionsYu Date: Tue, 28 Jul 2026 19:41:59 +0800 Subject: [PATCH 2/6] Wrap sibling SVG element layers uniformly as peer Groups during resolve to preserve source hierarchy. --- src/cli/CommandResolve.cpp | 70 +++++++++++++++----------------------- 1 file changed, 27 insertions(+), 43 deletions(-) diff --git a/src/cli/CommandResolve.cpp b/src/cli/CommandResolve.cpp index 604f014f14..af5875f720 100644 --- a/src/cli/CommandResolve.cpp +++ b/src/cli/CommandResolve.cpp @@ -30,7 +30,6 @@ #include "pagx/PAGXOptimizer.h" #include "pagx/nodes/Composition.h" #include "pagx/nodes/Group.h" -#include "pagx/nodes/LayoutNode.h" namespace pagx::cli { @@ -221,52 +220,37 @@ static bool ResolveOneLayer(Layer* layer, const std::string& baseDir, layer->contents.push_back(element); } } else if (canDowngradeAll) { - for (size_t i = 0; i < elementLayers.size(); i++) { - auto* elemLayer = elementLayers[i]; - bool unpackFirst = false; - if (i == 0 && elemLayer->matrix.isIdentity()) { - unpackFirst = true; - for (auto* child : elemLayer->contents) { - auto* layoutNode = LayoutNode::AsLayoutNode(child); - if (layoutNode != nullptr && - (!std::isnan(layoutNode->right) || !std::isnan(layoutNode->bottom) || - !std::isnan(layoutNode->centerX) || !std::isnan(layoutNode->centerY))) { - unpackFirst = false; - break; - } - } + // Wrap every element layer uniformly in its own Group so sibling shapes stay at the same + // depth (peer Groups), preserving the source hierarchy. Flattening only the first layer's + // contents while wrapping the rest would break that symmetry and is unnecessary — a trailing + // painter is isolated by its enclosing Group either way. This mirrors the uniform handling in + // PAGXOptimizer::DowngradeShellChildren. + for (auto* elemLayer : elementLayers) { + auto m = elemLayer->matrix; + bool hasSkew = !pag::FloatNearlyEqual(m.a * m.c + m.b * m.d, 0.0f); + if (hasSkew) { + // Matrix contains skew which cannot be represented by Group's + // position/scale/rotation. Keep it as a Layer instead of downgrading. + layer->children.push_back(elemLayer); + continue; } - if (unpackFirst) { - for (auto* element : elemLayer->contents) { - layer->contents.push_back(element); - } - } else { - auto m = elemLayer->matrix; - bool hasSkew = !pag::FloatNearlyEqual(m.a * m.c + m.b * m.d, 0.0f); - if (hasSkew) { - // Matrix contains skew which cannot be represented by Group's - // position/scale/rotation. Keep it as a Layer instead of downgrading. - layer->children.push_back(elemLayer); - continue; - } - auto* group = doc->makeNode(); - group->elements = std::move(elemLayer->contents); - if (!elemLayer->matrix.isIdentity()) { - group->position = {m.tx, m.ty}; - if (m.a != 1 || m.b != 0 || m.c != 0 || m.d != 1) { - float sx = std::sqrt(m.a * m.a + m.b * m.b); - float sy = std::sqrt(m.c * m.c + m.d * m.d); - float det = m.a * m.d - m.b * m.c; - if (det < 0) { - sy = -sy; - } - float rot = pag::RadiansToDegrees(std::atan2(m.b, m.a)); - group->scale = {sx, sy}; - group->rotation = rot; + auto* group = doc->makeNode(); + group->elements = std::move(elemLayer->contents); + if (!elemLayer->matrix.isIdentity()) { + group->position = {m.tx, m.ty}; + if (m.a != 1 || m.b != 0 || m.c != 0 || m.d != 1) { + float sx = std::sqrt(m.a * m.a + m.b * m.b); + float sy = std::sqrt(m.c * m.c + m.d * m.d); + float det = m.a * m.d - m.b * m.c; + if (det < 0) { + sy = -sy; } + float rot = pag::RadiansToDegrees(std::atan2(m.b, m.a)); + group->scale = {sx, sy}; + group->rotation = rot; } - layer->contents.push_back(group); } + layer->contents.push_back(group); } } else { for (auto* elemLayer : elementLayers) { From 03b1a7d26c7f819fb948a7407c49261d0c8c7a2f Mon Sep 17 00:00:00 2001 From: OnionsYu Date: Tue, 28 Jul 2026 20:00:40 +0800 Subject: [PATCH 3/6] Strengthen resolve and gradient tests to assert peer Group symmetry, extent-keyword radii, and transparent-stop RGB borrowing. --- test/src/PAGXCliTest.cpp | 19 ++++---- test/src/PAGXHTMLImporterTest.cpp | 75 +++++++++++++++++++++++++++---- 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/test/src/PAGXCliTest.cpp b/test/src/PAGXCliTest.cpp index 112380b824..06b3c9bb39 100644 --- a/test/src/PAGXCliTest.cpp +++ b/test/src/PAGXCliTest.cpp @@ -38,6 +38,7 @@ #include "pagx/PAGXExporter.h" #include "pagx/PAGXImporter.h" #include "pagx/nodes/Font.h" +#include "pagx/nodes/Group.h" #include "pagx/nodes/Image.h" #include "pagx/nodes/ImagePattern.h" #include "tgfx/core/Bitmap.h" @@ -2679,8 +2680,9 @@ CLI_TEST(PAGXCliTest, Resolve_MissingFile) { } CLI_TEST(PAGXCliTest, Resolve_MultiLayerPreservesIsolation) { - // Verifies that resolving an inline SVG with multiple elements preserves each SVG element - // in a separate painter scope, preventing painter accumulation bugs. + // Resolving an inline SVG with two sibling paths must preserve their common source depth: + // each path and its painter live in a separate peer Group. Flattening only the first path while + // grouping the second would make the output asymmetric and can change painter accumulation. auto pagxPath = CopyToTemp("import_resolve_multi_layer.pagx", "resolve_multi_layer.pagx"); auto ret = CallRun(pagx::cli::RunResolve, {"resolve", pagxPath}); EXPECT_EQ(ret, 0); @@ -2691,14 +2693,15 @@ CLI_TEST(PAGXCliTest, Resolve_MultiLayerPreservesIsolation) { ASSERT_EQ(doc->layers.size(), 1u); auto* hostLayer = doc->layers[0]; - EXPECT_FALSE(hostLayer->contents.empty()); - size_t groupCount = 0; + EXPECT_TRUE(hostLayer->children.empty()); + ASSERT_EQ(hostLayer->contents.size(), 2u); for (auto* element : hostLayer->contents) { - if (element->nodeType() == pagx::NodeType::Group) { - groupCount++; - } + ASSERT_EQ(element->nodeType(), pagx::NodeType::Group); + auto* group = static_cast(element); + ASSERT_EQ(group->elements.size(), 2u); + EXPECT_EQ(group->elements[0]->nodeType(), pagx::NodeType::Path); + EXPECT_EQ(group->elements[1]->nodeType(), pagx::NodeType::Stroke); } - EXPECT_GE(groupCount, 1u); // Screenshot test: render the resolved file and compare against baseline. EXPECT_TRUE(RenderAndCompare({"render", pagxPath}, "PAGXCliTest/ImportResolve_MultiLayer")); diff --git a/test/src/PAGXHTMLImporterTest.cpp b/test/src/PAGXHTMLImporterTest.cpp index 191ae520b0..7701680377 100644 --- a/test/src/PAGXHTMLImporterTest.cpp +++ b/test/src/PAGXHTMLImporterTest.cpp @@ -1164,10 +1164,45 @@ PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleOmittedSizeUsesFarthestCorner EXPECT_TRUE(NearlyEqual(rg->radius, 335.41f, 0.5f)); } -PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleClosestSideExtent) { +PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleExtentKeywordsUseDistanceFromCenter) { + struct ExtentCase { + const char* keyword; + float radius; + }; + // Center (100,50) in a 400x200 box gives side distances 100, 300, 50, 150. + // Corner radii are therefore hypot(100,50) and hypot(300,150). + const std::vector cases = { + {"closest-side", 50.0f}, + {"farthest-side", 300.0f}, + {"closest-corner", 111.80f}, + {"farthest-corner", 335.41f}, + }; + for (const auto& extent : cases) { + SCOPED_TRACE(extent.keyword); + auto html = std::string( + "" + "
"; + auto doc = ParseFromString(html); + ASSERT_NE(doc, nullptr); + auto* div = doc->layers.front()->children.front(); + auto* fill = FindElementOfType(div); + ASSERT_NE(fill, nullptr); + auto* rg = As(fill->color); + ASSERT_NE(rg, nullptr); + EXPECT_FALSE(rg->fitsToGeometry); + EXPECT_TRUE(NearlyEqual(rg->center.x, 100.0f, 0.01f)); + EXPECT_TRUE(NearlyEqual(rg->center.y, 50.0f, 0.01f)); + EXPECT_TRUE(NearlyEqual(rg->radius, extent.radius, 0.01f)); + } +} + +PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleExtentOnSquareBoxKeepsNormalised) { auto doc = ParseFromString(R"HTML( - -
+ +
)HTML"); ASSERT_NE(doc, nullptr); @@ -1176,12 +1211,34 @@ PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleClosestSideExtent) { ASSERT_NE(fill, nullptr); auto* rg = As(fill->color); ASSERT_NE(rg, nullptr); - // closest-side: center (100,100) on a 400x200 box; nearest edge distances are left=100, - // right=300, top=100, bottom=100, so the radius is 100px in the isotropic pixel model. - EXPECT_FALSE(rg->fitsToGeometry); - EXPECT_TRUE(NearlyEqual(rg->center.x, 100.0f, 0.5f)); - EXPECT_TRUE(NearlyEqual(rg->center.y, 100.0f, 0.5f)); - EXPECT_TRUE(NearlyEqual(rg->radius, 100.0f, 0.5f)); + // A square box is already isotropic in geometry space, so the farthest-corner radius + // hypot(50,50) is stored as hypot(0.5,0.5) instead of switching to pixel coordinates. + EXPECT_TRUE(rg->fitsToGeometry); + EXPECT_TRUE(NearlyEqual(rg->center.x, 0.5f, 0.01f)); + EXPECT_TRUE(NearlyEqual(rg->center.y, 0.5f, 0.01f)); + EXPECT_TRUE(NearlyEqual(rg->radius, 0.7071f, 0.001f)); +} + +PAG_TEST(PAGXHTMLImporterTest, GradientTransparentStopsBorrowOpaqueNeighborRGB) { + auto doc = ParseFromString(R"HTML( + +
+ + )HTML"); + ASSERT_NE(doc, nullptr); + auto* div = doc->layers.front()->children.front(); + auto* fill = FindElementOfType(div); + ASSERT_NE(fill, nullptr); + auto* lg = As(fill->color); + ASSERT_NE(lg, nullptr); + ASSERT_EQ(lg->colorStops.size(), 3u); + + // The renderer interpolates unpremultiplied colors. Giving both transparent edge stops the + // purple neighbor's RGB preserves a purple fade instead of blending through transparent black. + EXPECT_TRUE(ColorNear(lg->colorStops[0]->color, HexColor(0xDCD2FF, 0.0f))); + EXPECT_TRUE(ColorNear(lg->colorStops[1]->color, HexColor(0xDCD2FF, 0.4f))); + EXPECT_TRUE(ColorNear(lg->colorStops[2]->color, HexColor(0xDCD2FF, 0.0f))); } PAG_TEST(PAGXHTMLImporterTest, ConicGradientAngleOffset) { From 9cd196af4f262585d20f6e2cc34064bee1503c5f Mon Sep 17 00:00:00 2001 From: OnionsYu Date: Wed, 29 Jul 2026 14:54:01 +0800 Subject: [PATCH 4/6] Carry the CSS font-weight axis as a real-face style label instead of faux bold so imported headings and bold runs resolve the authored heavy face and preserve the SemiBold Bold Black distinction. --- src/pagx/html/importer/HTMLBoxAttributes.h | 17 +++++---- src/pagx/html/importer/HTMLStyleCascade.cpp | 10 ++--- .../html/importer/HTMLTextFragmentBuilder.h | 9 +++-- src/pagx/utils/CSSFontStyle.cpp | 20 +++++----- src/pagx/utils/CSSFontStyle.h | 37 +++++++++---------- test/src/PAGXCSSFontStyleTest.cpp | 22 +++++------ test/src/PAGXHTMLImporterTest.cpp | 28 +++++++------- 7 files changed, 71 insertions(+), 72 deletions(-) diff --git a/src/pagx/html/importer/HTMLBoxAttributes.h b/src/pagx/html/importer/HTMLBoxAttributes.h index a100c06fc7..35d380c62d 100644 --- a/src/pagx/html/importer/HTMLBoxAttributes.h +++ b/src/pagx/html/importer/HTMLBoxAttributes.h @@ -40,8 +40,8 @@ static constexpr const char* HTML_DEFAULT_FONT_FAMILY = "Arial"; /** * Default font style/variant name written to every imported `Text` node. The synthesis in - * `ResolveFontStyleSynthesis` leaves the style label empty for the plain base face (bold / italic - * are carried by `fauxBold` / `fauxItalic`); this constant substitutes the canonical "Regular" + * `ResolveFontStyleSynthesis` leaves the style label empty for the plain Regular-weight upright + * face (italic is carried by `fauxItalic`); this constant substitutes the canonical "Regular" * name so every HTML-imported `Text` node always carries a concrete `fontStyle`. */ static constexpr const char* HTML_DEFAULT_FONT_STYLE = "Regular"; @@ -74,12 +74,13 @@ struct HTMLInheritedStyle { std::string fontSize = {}; std::string fontWeight = {}; std::string fontStyle = {}; - std::string fontStyleName = {}; // real-face style label, e.g. "Light" / "Medium" / "" - // Synthetic weight / slant the renderer must emboss on top of the resolved face. Set by - // `resolveInheritedStyle` for bold (CSS weight >= 600) and italic/oblique requests whose axis is - // dropped from `fontStyleName` (see `ResolveFontStyleSynthesis`). Carried through to - // `Text::fauxBold` / `Text::fauxItalic` so authored weight / slant survives even when the styled - // web face is not installed on the render host. + std::string fontStyleName = {}; // real-face style label, e.g. "Light" / "Bold" / "Black" / "" + // Synthetic slant the renderer must emboss on top of the resolved face. Set by + // `resolveInheritedStyle` for italic/oblique requests, whose axis is dropped from `fontStyleName` + // (see `ResolveFontStyleSynthesis`) and carried through to `Text::fauxItalic` so the authored + // slant survives even when the styled italic face is not installed on the render host. The weight + // axis is never synthesised (it stays in `fontStyleName` as a real-face keyword), so `fauxBold` + // is always false here. bool fauxBold = false; bool fauxItalic = false; std::string letterSpacing = {}; diff --git a/src/pagx/html/importer/HTMLStyleCascade.cpp b/src/pagx/html/importer/HTMLStyleCascade.cpp index a6f1a7cbdb..9e2161f778 100644 --- a/src/pagx/html/importer/HTMLStyleCascade.cpp +++ b/src/pagx/html/importer/HTMLStyleCascade.cpp @@ -545,11 +545,11 @@ HTMLInheritedStyle HTMLStyleCascade::resolveInheritedStyle(const std::shared_ptr out.textFillImage = ownBgImage; } // Split the CSS font-weight / font-style request into the real-face style label PAGX Text - // resolves plus the synthetic (faux) axes the renderer embosses on top. Bold (weight >= 600) and - // italic/oblique are baked as faux flags and dropped from the label so an uninstalled web face - // (e.g. "Noto Sans SC Black Italic") still renders at the authored weight and slant instead of - // collapsing to a thin upright fallback. Lighter weights (Light / Medium) cannot be synthesised - // and stay in the label. + // resolves plus the synthetic (faux) italic axis the renderer embosses on top. The weight axis + // is always written as a real-face keyword (Bold / SemiBold / Black) so the renderer resolves the + // authored heavy face when it is installed or embedded and preserves the SemiBold / Bold / Black + // distinction, falling back to host faux emboldening only for a missing face. Italic stays a faux + // flag so an oblique slant survives when the styled italic face is unavailable. FontStyleSynthesis fontSynthesis = ResolveFontStyleSynthesis(out.fontWeight, out.fontStyle); out.fontStyleName = fontSynthesis.fontStyleName; out.fauxBold = fontSynthesis.fauxBold; diff --git a/src/pagx/html/importer/HTMLTextFragmentBuilder.h b/src/pagx/html/importer/HTMLTextFragmentBuilder.h index 47ee271639..2f21d2c639 100644 --- a/src/pagx/html/importer/HTMLTextFragmentBuilder.h +++ b/src/pagx/html/importer/HTMLTextFragmentBuilder.h @@ -58,10 +58,11 @@ class HTMLTextFragmentBuilder { struct TextFragment { std::string text = {}; std::string fontFamily = {}; - std::string fontStyleName = {}; // real-face style label, e.g. "Light" / "Medium" / "" - // Synthetic weight / slant baked in from the CSS request (see `ResolveFontStyleSynthesis`). - // Surface as `Text::fauxBold` / `Text::fauxItalic` so authored bold / italic survives a - // missing styled face on the render host. + std::string fontStyleName = {}; // real-face style label, e.g. "Light" / "Bold" / "Black" / "" + // Synthetic slant baked in from the CSS request (see `ResolveFontStyleSynthesis`). Surfaces as + // `Text::fauxItalic` so an authored oblique slant survives a missing styled italic face on the + // render host. The weight axis is never synthesised (it stays in `fontStyleName`), so + // `fauxBold` is always false. bool fauxBold = false; bool fauxItalic = false; float fontSize = HTML_DEFAULT_FONT_SIZE; diff --git a/src/pagx/utils/CSSFontStyle.cpp b/src/pagx/utils/CSSFontStyle.cpp index 050aa068e8..4c41293a77 100644 --- a/src/pagx/utils/CSSFontStyle.cpp +++ b/src/pagx/utils/CSSFontStyle.cpp @@ -134,20 +134,18 @@ FontStyleSynthesis ResolveFontStyleSynthesis(const std::string& cssFontWeight, const std::string& cssFontStyle) { FontStyleSynthesis out; int numericWeight = CssFontWeightToNumeric(cssFontWeight); - bool italic = IsItalicCssStyle(cssFontStyle); - // Threshold mirrors CSS bold synthesis: SemiBold-or-heavier (>= 600) is treated as a faux-bold - // request because the renderer's faux emboldening is a single fixed step and cannot distinguish - // SemiBold from Black anyway. - out.fauxBold = numericWeight >= 600; - out.fauxItalic = italic; - // Keep only the real-face axes in the style label: a synthesised axis is dropped so the renderer - // resolves a base (Regular / lighter) face and faux adds the missing weight / slant on top, - // instead of resolving the styled face and doubling up. - const char* weightKeyword = - out.fauxBold ? nullptr : WeightKeywordForRoundedHundreds(numericWeight); + // The weight axis is always carried as a real-face style label (Bold / SemiBold / Black / etc.), + // not synthesised: this lets the renderer resolve the authored heavy face when it is installed or + // embedded, and only fall back to faux emboldening (applied by the render host on a missing face) + // without collapsing SemiBold / Bold / Black into one indistinguishable step. + out.fauxBold = false; + const char* weightKeyword = WeightKeywordForRoundedHundreds(numericWeight); if (weightKeyword) { out.fontStyleName = weightKeyword; } + // Italic is kept as a faux axis: an oblique slant can be synthesised on top of any upright face, + // so it survives even when the styled italic face is unavailable. + out.fauxItalic = IsItalicCssStyle(cssFontStyle); return out; } diff --git a/src/pagx/utils/CSSFontStyle.h b/src/pagx/utils/CSSFontStyle.h index 961bffb647..c36b45d41d 100644 --- a/src/pagx/utils/CSSFontStyle.h +++ b/src/pagx/utils/CSSFontStyle.h @@ -37,27 +37,26 @@ namespace pagx { std::string ResolveFontStyleName(const std::string& cssFontWeight, const std::string& cssFontStyle); // Splits a CSS font-weight / font-style request into the real face style the renderer should -// resolve plus the synthetic (faux) axes it must emboss on top. This lets an importer bake the -// authored weight / slant into a `.pagx` without relying on render-time face introspection: -// uninstalled web faces (the common case for HTML / SVG imports such as "Noto Sans SC Black -// Italic") still render at the authored weight and slant via faux synthesis. +// resolve plus the synthetic (faux) italic axis it may emboss on top. // -// The synthesised axes are *removed* from `fontStyleName` rather than kept alongside the faux -// flags. A host that does ship the real heavy / italic face must not be emboldened twice -// (faux-on-top-of-real, which the renderer layers additively), and keeping the styled name would -// trigger exactly that whenever the styled face is resolvable. Stripping it makes the rendered -// result identical whether or not the styled face is installed. Weights below the bold threshold -// (Thin / ExtraLight / Light / Medium) cannot be synthesised — faux only adds weight, never -// removes it — so those keywords are preserved in `fontStyleName` and carry no faux flag. +// The weight axis is always emitted as a real-face style label (Bold / SemiBold / Black / etc. per +// the numeric weight rounded to the nearest hundred; 400 leaves the weight portion empty), never as +// a faux flag. This lets the renderer resolve the authored heavy face when it is installed or +// embedded (preserving the distinction between SemiBold, Bold and Black) and only fall back to host +// faux emboldening for a genuinely missing face, instead of always synthesising a single fixed step +// on a Regular base. // -// Examples (threshold mirrors CSS bold synthesis at weight >= 600): -// ("900", "italic") -> {fontStyleName: "", fauxBold: true, fauxItalic: true} -// ("700", "") -> {fontStyleName: "", fauxBold: true, fauxItalic: false} -// ("600", "italic") -> {fontStyleName: "", fauxBold: true, fauxItalic: true} -// ("500", "italic") -> {fontStyleName: "Medium", fauxBold: false, fauxItalic: true} -// ("300", "") -> {fontStyleName: "Light", fauxBold: false, fauxItalic: false} -// ("400", "italic") -> {fontStyleName: "", fauxBold: false, fauxItalic: true} -// ("400", "") -> {fontStyleName: "", fauxBold: false, fauxItalic: false} +// Italic stays a synthetic axis (`fauxItalic`): an oblique slant can be synthesised on top of any +// upright face, so it survives even when the styled italic face is unavailable. +// +// Examples: +// ("900", "italic") -> {fontStyleName: "Black", fauxBold: false, fauxItalic: true} +// ("700", "") -> {fontStyleName: "Bold", fauxBold: false, fauxItalic: false} +// ("600", "italic") -> {fontStyleName: "SemiBold", fauxBold: false, fauxItalic: true} +// ("500", "italic") -> {fontStyleName: "Medium", fauxBold: false, fauxItalic: true} +// ("300", "") -> {fontStyleName: "Light", fauxBold: false, fauxItalic: false} +// ("400", "italic") -> {fontStyleName: "", fauxBold: false, fauxItalic: true} +// ("400", "") -> {fontStyleName: "", fauxBold: false, fauxItalic: false} struct FontStyleSynthesis { std::string fontStyleName = {}; bool fauxBold = false; diff --git a/test/src/PAGXCSSFontStyleTest.cpp b/test/src/PAGXCSSFontStyleTest.cpp index de61886567..be452368de 100644 --- a/test/src/PAGXCSSFontStyleTest.cpp +++ b/test/src/PAGXCSSFontStyleTest.cpp @@ -82,27 +82,27 @@ CLI_TEST(PAGXCSSFontStyleTest, ResolveName_WhitespaceAndCaseNormalised) { EXPECT_EQ(pagx::ResolveFontStyleName(" BOLD ", " ITALIC "), "Bold Italic"); } -// ResolveFontStyleSynthesis: splits the request into a real-face label plus faux axes. The bold -// synthesis threshold is weight >= 600; weights below stay as keywords with no faux flag. +// ResolveFontStyleSynthesis: splits the request into a real-face label plus a faux italic axis. The +// weight axis is always a real-face keyword (never faux); only italic is synthesised. -CLI_TEST(PAGXCSSFontStyleTest, Synthesis_BlackItalicIsAllFaux) { +CLI_TEST(PAGXCSSFontStyleTest, Synthesis_BlackItalicKeepsRealWeightFauxItalic) { auto out = pagx::ResolveFontStyleSynthesis("900", "italic"); - EXPECT_EQ(out.fontStyleName, ""); - EXPECT_TRUE(out.fauxBold); + EXPECT_EQ(out.fontStyleName, "Black"); + EXPECT_FALSE(out.fauxBold); EXPECT_TRUE(out.fauxItalic); } -CLI_TEST(PAGXCSSFontStyleTest, Synthesis_BoldOnly) { +CLI_TEST(PAGXCSSFontStyleTest, Synthesis_BoldKeepsRealWeight) { auto out = pagx::ResolveFontStyleSynthesis("700", ""); - EXPECT_EQ(out.fontStyleName, ""); - EXPECT_TRUE(out.fauxBold); + EXPECT_EQ(out.fontStyleName, "Bold"); + EXPECT_FALSE(out.fauxBold); EXPECT_FALSE(out.fauxItalic); } -CLI_TEST(PAGXCSSFontStyleTest, Synthesis_SemiBoldThresholdIsFauxBold) { +CLI_TEST(PAGXCSSFontStyleTest, Synthesis_SemiBoldKeepsRealWeightFauxItalic) { auto out = pagx::ResolveFontStyleSynthesis("600", "italic"); - EXPECT_EQ(out.fontStyleName, ""); - EXPECT_TRUE(out.fauxBold); + EXPECT_EQ(out.fontStyleName, "SemiBold"); + EXPECT_FALSE(out.fauxBold); EXPECT_TRUE(out.fauxItalic); } diff --git a/test/src/PAGXHTMLImporterTest.cpp b/test/src/PAGXHTMLImporterTest.cpp index 7701680377..87504ef071 100644 --- a/test/src/PAGXHTMLImporterTest.cpp +++ b/test/src/PAGXHTMLImporterTest.cpp @@ -2969,16 +2969,16 @@ PAG_TEST(PAGXHTMLImporterTest, HeadingDefaultFontSizes) { auto* text = FindElementOfType(leaf); ASSERT_NE(text, nullptr) << r.tag; EXPECT_FLOAT_EQ(text->fontSize, r.size) << r.tag; - // Headings default to font-weight:bold, which is baked as faux bold (synthesised on a base - // face) rather than a "Bold" style label so the authored weight survives a missing styled face. - // The base-face label surfaces as the canonical "Regular" so every Text node carries a style. - EXPECT_EQ(text->fontStyle, "Regular") << r.tag; - EXPECT_TRUE(text->fauxBold) << r.tag; + // Headings default to font-weight:bold, which is written as a real-face "Bold" style label so + // the renderer resolves the authored heavy face (falling back to host faux emboldening only for + // a missing face). No faux flag is baked into the Text node. + EXPECT_EQ(text->fontStyle, "Bold") << r.tag; + EXPECT_FALSE(text->fauxBold) << r.tag; EXPECT_FALSE(text->fauxItalic) << r.tag; } } -PAG_TEST(PAGXHTMLImporterTest, FontWeightNumericMapsToFauxBold) { +PAG_TEST(PAGXHTMLImporterTest, FontWeightNumericMapsToBold) { auto doc = ParseFromString(R"HTML( Heavy @@ -2987,10 +2987,10 @@ PAG_TEST(PAGXHTMLImporterTest, FontWeightNumericMapsToFauxBold) { ASSERT_NE(doc, nullptr); auto* text = FindElementOfType(doc->layers.front()->children.front()); ASSERT_NE(text, nullptr); - // Bold (weight >= 600) is synthesised via faux bold rather than locking onto a "Bold" face; the - // base-face label surfaces as the canonical "Regular". - EXPECT_EQ(text->fontStyle, "Regular"); - EXPECT_TRUE(text->fauxBold); + // Weight 700 is written as a real-face "Bold" style label so the renderer can resolve the + // authored heavy face; no faux flag is baked in. + EXPECT_EQ(text->fontStyle, "Bold"); + EXPECT_FALSE(text->fauxBold); EXPECT_FALSE(text->fauxItalic); } @@ -3018,10 +3018,10 @@ PAG_TEST(PAGXHTMLImporterTest, BoldItalicCombined) { ASSERT_NE(doc, nullptr); auto* text = FindElementOfType(doc->layers.front()->children.front()); ASSERT_NE(text, nullptr); - // Both axes are synthesised: the style label drops to the canonical "Regular" base face and both - // faux flags are set so the run renders bold + italic even when the styled face is unavailable. - EXPECT_EQ(text->fontStyle, "Regular"); - EXPECT_TRUE(text->fauxBold); + // The weight axis becomes a real-face "Bold" style label; only the italic axis is synthesised via + // faux italic so the slant survives a missing styled italic face. + EXPECT_EQ(text->fontStyle, "Bold"); + EXPECT_FALSE(text->fauxBold); EXPECT_TRUE(text->fauxItalic); } From c90b3e495f52fa2fb82be90bcc3596df21cb0d9e Mon Sep 17 00:00:00 2001 From: OnionsYu Date: Wed, 29 Jul 2026 17:20:19 +0800 Subject: [PATCH 5/6] Stop emitting redundant visible false on HTML importer mask layers. --- src/pagx/html/importer/HTMLElementEmitter.cpp | 16 +++++++--------- src/pagx/html/importer/HTMLParserContext.h | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/pagx/html/importer/HTMLElementEmitter.cpp b/src/pagx/html/importer/HTMLElementEmitter.cpp index 3d880adf01..8544994988 100644 --- a/src/pagx/html/importer/HTMLElementEmitter.cpp +++ b/src/pagx/html/importer/HTMLElementEmitter.cpp @@ -697,8 +697,8 @@ void HTMLParserContext::applyMaskOrClip(Layer* layer, const HTMLBoxAttributes& b float intrinsicH = svgDoc->height; // The mask SVG produces one or more content layers (a single drawn shape in the common case). - // Wrap them under one invisible, layout-excluded mask layer so a multi-shape clip-path also - // works, then transplant every node from the temporary SVG document into ours. + // Wrap them under one layout-excluded mask layer so a multi-shape clip-path also works, then + // transplant every node from the temporary SVG document into ours. Layer* maskLayer = nullptr; if (svgDoc->layers.size() == 1) { maskLayer = svgDoc->layers[0]; @@ -708,7 +708,6 @@ void HTMLParserContext::applyMaskOrClip(Layer* layer, const HTMLBoxAttributes& b maskLayer->children.push_back(l); } } - maskLayer->visible = false; maskLayer->includeInLayout = false; // CSS `mask-size` / `mask-position` scale and offset the intrinsic mask box onto the masked // element; replay that transform onto the mask layer. Contour clip-paths are framed to the box @@ -730,9 +729,9 @@ void HTMLParserContext::applyMaskOrClip(Layer* layer, const HTMLBoxAttributes& b layer->mask = maskLayer; layer->maskType = maskType; // The mask layer must be reachable in the display list for the renderer's mask lookup, and must - // share the masked layer's local coordinate origin. Adding it as an invisible, layout-excluded - // child satisfies both: it is walked by LayerBuilder but neither drawn (maskOwner is set) nor - // laid out (includeInLayout is false). + // share the masked layer's local coordinate origin. Adding it as a layout-excluded child + // satisfies both: it is walked by LayerBuilder but neither drawn (maskOwner is set on the tgfx + // side) nor laid out (includeInLayout is false). layer->children.push_back(maskLayer); } @@ -767,7 +766,6 @@ void HTMLParserContext::applyRoundedOverflowClip(Layer* layer, const HTMLBoxAttr // resolved by layout. A Contour mask reads only the shape's coverage, clipping descendants to // the rounded outline instead of the layer's rectangle. auto* maskLayer = _document->makeNode(); - maskLayer->visible = false; maskLayer->includeInLayout = false; maskLayer->percentWidth = 100.0f; maskLayer->percentHeight = 100.0f; @@ -782,8 +780,8 @@ void HTMLParserContext::applyRoundedOverflowClip(Layer* layer, const HTMLBoxAttr // The rounded mask now performs the clip; drop the rectangular scrollRect so it does not also // square off the corners the mask just rounded. layer->clipToBounds = false; - // Invisible, layout-excluded child so it shares the masked layer's local coordinate origin and - // stays reachable by the renderer's mask lookup (mirrors `applyMaskOrClip`). + // Layout-excluded child so it shares the masked layer's local coordinate origin and stays + // reachable by the renderer's mask lookup (mirrors `applyMaskOrClip`). layer->children.push_back(maskLayer); } diff --git a/src/pagx/html/importer/HTMLParserContext.h b/src/pagx/html/importer/HTMLParserContext.h index ded413402f..9e0a6dd290 100644 --- a/src/pagx/html/importer/HTMLParserContext.h +++ b/src/pagx/html/importer/HTMLParserContext.h @@ -124,7 +124,7 @@ class HTMLParserContext { // equivalent SVG geometry — and attaches it to `layer` as `layer->mask` / `maskType` // (the inverse of `HTMLWriter::writeMaskCSS` / `writeClipDef`). The mask geometry SVG is parsed // through `SVGImporter`, and its nodes are transplanted into `_document`. The mask layer is added - // as an invisible, layout-excluded child of `layer` so it shares the masked layer's local + // as a layout-excluded child of `layer` so it shares the masked layer's local // coordinate space and is reachable by the renderer's mask lookup. No-op when the box carries // neither a mask nor a clip-path reference. `box` supplies the masked layer's resolved size used // to frame a contour clip-path SVG. From 4a207be1d9cc823a18ec730040ef54444efb5c91 Mon Sep 17 00:00:00 2001 From: OnionsYu Date: Fri, 31 Jul 2026 16:46:29 +0800 Subject: [PATCH 6/6] Downgrade transformable sibling layers to peer Groups with scale-independent skew detection and fix radial-gradient circle extent radii for centers outside the box. --- .../cli/import_resolve_skew_siblings.pagx | 8 ++++ .../import_resolve_transformed_siblings.pagx | 9 +++++ src/cli/CommandResolve.cpp | 32 ++++++++++----- src/pagx/html/importer/HTMLValueParser.cpp | 20 +++++----- test/src/PAGXCliTest.cpp | 40 +++++++++++++++++++ test/src/PAGXHTMLImporterTest.cpp | 30 ++++++++++++-- 6 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 resources/cli/import_resolve_skew_siblings.pagx create mode 100644 resources/cli/import_resolve_transformed_siblings.pagx diff --git a/resources/cli/import_resolve_skew_siblings.pagx b/resources/cli/import_resolve_skew_siblings.pagx new file mode 100644 index 0000000000..baf214301b --- /dev/null +++ b/resources/cli/import_resolve_skew_siblings.pagx @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/resources/cli/import_resolve_transformed_siblings.pagx b/resources/cli/import_resolve_transformed_siblings.pagx new file mode 100644 index 0000000000..c135537621 --- /dev/null +++ b/resources/cli/import_resolve_transformed_siblings.pagx @@ -0,0 +1,9 @@ + + + + + + + + diff --git a/src/cli/CommandResolve.cpp b/src/cli/CommandResolve.cpp index af5875f720..1ff13635f3 100644 --- a/src/cli/CommandResolve.cpp +++ b/src/cli/CommandResolve.cpp @@ -124,6 +124,20 @@ static int ParseResolveOptions(int argc, char* argv[], ResolveOptions* options) // Resolve logic //-------------------------------------------------------------------------------------------------- +// A Group's position/scale/rotation fields can represent a matrix whose axes remain orthogonal. +// Compare the normalized axis dot product so the result does not depend on the matrix scale. +// Degenerate axes are kept as Layers because the decomposition below cannot recover their rotation. +static bool CanDowngradeMatrixToGroup(const Matrix& matrix) { + float xLength = std::hypot(matrix.a, matrix.b); + float yLength = std::hypot(matrix.c, matrix.d); + if (pag::FloatNearlyZero(xLength) || pag::FloatNearlyZero(yLength)) { + return false; + } + float normalizedDot = + (matrix.a / xLength) * (matrix.c / yLength) + (matrix.b / xLength) * (matrix.d / yLength); + return pag::FloatNearlyZero(normalizedDot); +} + static bool ResolveOneLayer(Layer* layer, const std::string& baseDir, const ImportFormatOptions& formatOptions, PAGXDocument* doc) { bool hasImportSource = !layer->importDirective.source.empty(); @@ -208,7 +222,8 @@ static bool ResolveOneLayer(Layer* layer, const std::string& baseDir, } else if (elementLayers.size() > 1) { canDowngradeAll = true; for (auto* el : elementLayers) { - if (!el->children.empty() || HasLayerOnlyFeatures(el)) { + if (!el->children.empty() || HasLayerOnlyFeatures(el) || + !CanDowngradeMatrixToGroup(el->matrix)) { canDowngradeAll = false; break; } @@ -223,19 +238,18 @@ static bool ResolveOneLayer(Layer* layer, const std::string& baseDir, // Wrap every element layer uniformly in its own Group so sibling shapes stay at the same // depth (peer Groups), preserving the source hierarchy. Flattening only the first layer's // contents while wrapping the rest would break that symmetry and is unnecessary — a trailing - // painter is isolated by its enclosing Group either way. This mirrors the uniform handling in - // PAGXOptimizer::DowngradeShellChildren. + // painter is isolated by its enclosing Group either way. Structurally, this uses the same + // all-siblings Group strategy as PAGXOptimizer::DowngradeShellChildren, while also supporting + // transformable non-identity matrices here. for (auto* elemLayer : elementLayers) { - auto m = elemLayer->matrix; - bool hasSkew = !pag::FloatNearlyEqual(m.a * m.c + m.b * m.d, 0.0f); - if (hasSkew) { - // Matrix contains skew which cannot be represented by Group's - // position/scale/rotation. Keep it as a Layer instead of downgrading. - layer->children.push_back(elemLayer); + if (elemLayer->contents.empty() && elemLayer->customData.empty()) { continue; } + auto m = elemLayer->matrix; auto* group = doc->makeNode(); group->elements = std::move(elemLayer->contents); + group->customData = std::move(elemLayer->customData); + group->sourceLine = elemLayer->sourceLine; if (!elemLayer->matrix.isIdentity()) { group->position = {m.tx, m.ty}; if (m.a != 1 || m.b != 0 || m.c != 0 || m.d != 1) { diff --git a/src/pagx/html/importer/HTMLValueParser.cpp b/src/pagx/html/importer/HTMLValueParser.cpp index 767f4391e0..48b65de492 100644 --- a/src/pagx/html/importer/HTMLValueParser.cpp +++ b/src/pagx/html/importer/HTMLValueParser.cpp @@ -643,10 +643,10 @@ bool IsRadialExtentKeyword(const std::string& token) { // edges. float CircleExtentRadiusPx(const std::string& keyword, float cxPx, float cyPx, float boxWidth, float boxHeight) { - float left = cxPx; - float right = boxWidth - cxPx; - float top = cyPx; - float bottom = boxHeight - cyPx; + float left = std::abs(cxPx); + float right = std::abs(boxWidth - cxPx); + float top = std::abs(cyPx); + float bottom = std::abs(boxHeight - cyPx); float dx = std::max(left, right); float dy = std::max(top, bottom); if (keyword == "closest-side") { @@ -961,10 +961,11 @@ void HTMLValueParser::parseRadialDescriptor(const std::string& descriptor, float } } - // A circle keeps one uniform radius; an ellipse (explicit, or the shapeless default) stretches - // per axis. Detected up front because extent-keyword and omitted sizes resolve the radius from - // the center, which the position pass below establishes first. - bool isCircle = explicitCircle || (!explicitEllipse && sizeTokens.size() == 1); + // A single explicit length implies a circle. An extent keyword without a shape still uses CSS's + // default ellipse, so it must not enter the circle-only pixel-radius path below. + bool implicitCircle = + !explicitEllipse && sizeTokens.size() == 1 && !IsRadialExtentKeyword(sizeTokens[0]); + bool isCircle = explicitCircle || implicitCircle; // Position: `at `. Axis-locked keywords (left/right -> x, top/bottom -> y) are assigned // first so author order is irrelevant (`at top left` == `at left top`); the remaining `center` @@ -1156,7 +1157,8 @@ bool HTMLValueParser::finaliseGradientStops(GradientStops& stops) { // interpolates unpremultiplied, where a keyword `transparent` carries black RGB and would drag // the ramp toward grey/black. Rewrite each fully transparent stop's RGB to that of its nearest // opaque neighbour (alpha kept at 0) so the unpremultiplied interpolation matches CSS. A stop - // between two opaque colours prefers the earlier neighbour, matching the premultiplied midpoint. + // between two opaque colours prefers the earlier neighbour to avoid tinting the visible, + // higher-alpha side of the fade. for (size_t i = 0; i < stops.size(); ++i) { if (stops[i].second.alpha > 0.0f) continue; size_t donor = stops.size(); diff --git a/test/src/PAGXCliTest.cpp b/test/src/PAGXCliTest.cpp index 06b3c9bb39..901761914e 100644 --- a/test/src/PAGXCliTest.cpp +++ b/test/src/PAGXCliTest.cpp @@ -2707,6 +2707,46 @@ CLI_TEST(PAGXCliTest, Resolve_MultiLayerPreservesIsolation) { EXPECT_TRUE(RenderAndCompare({"render", pagxPath}, "PAGXCliTest/ImportResolve_MultiLayer")); } +CLI_TEST(PAGXCliTest, Resolve_SkewSiblingKeepsAllLayers) { + // The first sibling has both a tiny scale and a real skew. Skew detection must be + // scale-independent, and one non-downgradable matrix must keep every sibling in children so + // contents/children paint ordering cannot reverse them. + auto pagxPath = CopyToTemp("import_resolve_skew_siblings.pagx", "resolve_skew_siblings.pagx"); + auto ret = CallRun(pagx::cli::RunResolve, {"resolve", pagxPath}); + EXPECT_EQ(ret, 0); + + auto doc = pagx::PAGXImporter::FromFile(pagxPath); + ASSERT_NE(doc, nullptr); + ASSERT_EQ(doc->layers.size(), 1u); + auto* hostLayer = doc->layers[0]; + EXPECT_TRUE(hostLayer->contents.empty()); + ASSERT_EQ(hostLayer->children.size(), 2u); + EXPECT_FALSE(hostLayer->children[0]->matrix.isIdentity()); + EXPECT_TRUE(hostLayer->children[1]->matrix.isIdentity()); +} + +CLI_TEST(PAGXCliTest, Resolve_TransformableSiblingMatricesBecomeGroups) { + auto pagxPath = + CopyToTemp("import_resolve_transformed_siblings.pagx", "resolve_transformed_siblings.pagx"); + auto ret = CallRun(pagx::cli::RunResolve, {"resolve", pagxPath}); + EXPECT_EQ(ret, 0); + + auto doc = pagx::PAGXImporter::FromFile(pagxPath); + ASSERT_NE(doc, nullptr); + ASSERT_EQ(doc->layers.size(), 1u); + auto* hostLayer = doc->layers[0]; + EXPECT_TRUE(hostLayer->children.empty()); + ASSERT_EQ(hostLayer->contents.size(), 2u); + ASSERT_EQ(hostLayer->contents[0]->nodeType(), pagx::NodeType::Group); + auto* transformed = static_cast(hostLayer->contents[0]); + EXPECT_FLOAT_EQ(transformed->position.x, 4.0f); + EXPECT_FLOAT_EQ(transformed->position.y, 5.0f); + EXPECT_FLOAT_EQ(transformed->rotation, 30.0f); + EXPECT_FLOAT_EQ(transformed->scale.x, 2.0f); + EXPECT_FLOAT_EQ(transformed->scale.y, 3.0f); + EXPECT_EQ(hostLayer->contents[1]->nodeType(), pagx::NodeType::Group); +} + CLI_TEST(PAGXCliTest, Resolve_DeduplicatesInlineSvgImageIds) { // Each inline is imported by its own SVGImporter, which restarts auto-generated id // numbering from scratch, so two unrelated elements both become Image id="image1". diff --git a/test/src/PAGXHTMLImporterTest.cpp b/test/src/PAGXHTMLImporterTest.cpp index 87504ef071..a1189109e3 100644 --- a/test/src/PAGXHTMLImporterTest.cpp +++ b/test/src/PAGXHTMLImporterTest.cpp @@ -1199,6 +1199,27 @@ PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleExtentKeywordsUseDistanceFrom } } +PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleClosestSideOutsideBoxUsesPositiveDistance) { + auto doc = ParseFromString(R"HTML( + +
+ + )HTML"); + ASSERT_NE(doc, nullptr); + auto* div = doc->layers.front()->children.front(); + auto* fill = FindElementOfType(div); + ASSERT_NE(fill, nullptr); + auto* rg = As(fill->color); + ASSERT_NE(rg, nullptr); + // The center is outside the box, but closest-side is still the positive geometric distance to + // the nearest edge: min(80, 480, 138, 338) = 80px. + EXPECT_FALSE(rg->fitsToGeometry); + EXPECT_TRUE(NearlyEqual(rg->center.x, -80.0f, 0.01f)); + EXPECT_TRUE(NearlyEqual(rg->center.y, -138.0f, 0.01f)); + EXPECT_TRUE(NearlyEqual(rg->radius, 80.0f, 0.01f)); +} + PAG_TEST(PAGXHTMLImporterTest, RadialGradientCircleExtentOnSquareBoxKeepsNormalised) { auto doc = ParseFromString(R"HTML( @@ -1327,8 +1348,9 @@ PAG_TEST(PAGXHTMLImporterTest, RoundedOverflowClipUsesContourMaskForNonImageChil // `border-radius: 50%` inscribes an ellipse, so the mask geometry is an Ellipse. auto* ellipse = FindElementOfType(wrapper->mask); EXPECT_NE(ellipse, nullptr); - // The mask is an invisible, layout-excluded child that shares the container's coordinate space. - EXPECT_FALSE(wrapper->mask->visible); + // The mask stays visible at the PAGX level so the renderer can resolve it; mask ownership + // suppresses normal drawing. It is excluded only from layout. + EXPECT_TRUE(wrapper->mask->visible); EXPECT_FALSE(wrapper->mask->includeInLayout); } @@ -8125,7 +8147,7 @@ PAG_TEST(PAGXHTMLImporterTest, BackgroundDataImageExplicitSizeUsesNativeDimensio // CSS `mask-image: url(data:image/svg+xml,...)` with `mask-mode: alpha` rebuilds an alpha mask // layer (the inverse of HTMLWriter::writeMaskCSS). The ellipse geometry round-trips and the mask -// layer is attached invisibly and excluded from layout. +// layer stays renderer-visible for mask lookup and is excluded from layout. PAG_TEST(PAGXHTMLImporterTest, MaskImageAlphaRebuildsMaskLayer) { auto doc = ParseFromString(R"HTML( @@ -8138,7 +8160,7 @@ PAG_TEST(PAGXHTMLImporterTest, MaskImageAlphaRebuildsMaskLayer) { auto* masked = doc->layers.front()->children.front(); ASSERT_NE(masked->mask, nullptr); EXPECT_EQ(masked->maskType, pagx::MaskType::Alpha); - EXPECT_FALSE(masked->mask->visible); + EXPECT_TRUE(masked->mask->visible); EXPECT_FALSE(masked->mask->includeInLayout); auto* ellipse = FindElementOfType(masked->mask); ASSERT_NE(ellipse, nullptr);