Skip to content

Fix percentage opacity parsing during SVG load#520

Merged
wieslawsoltes merged 6 commits into
masterfrom
fix/opacity-percent-parsing
May 1, 2026
Merged

Fix percentage opacity parsing during SVG load#520
wieslawsoltes merged 6 commits into
masterfrom
fix/opacity-percent-parsing

Conversation

@wieslawsoltes
Copy link
Copy Markdown
Owner

@wieslawsoltes wieslawsoltes commented Apr 23, 2026

PR Summary: Handle Percentage Opacity Load Failures

Overview

This change fixes SVG load failures caused by percentage-valued opacity declarations such as
opacity="100%" while preserving existing SVG 1.1 / resvg behavior for invalid percentage
opacity values.

The reported failure came through the Avalonia integration path (SvgSource.LoadFromSvg(...)),
but the root cause was lower in the shared compatibility loader: staged opacity declarations were
eventually forwarded into the generated float-conversion path, which cannot parse % tokens.

Problem

The failing SVG used:

<svg xmlns="http://www.w3.org/2000/svg" ... opacity="100%">

During style flushing, SvgElementFactory.SetPropertyValue(...) forwarded that raw value to the
generated descriptor pipeline. Opacity-backed descriptors expect a plain numeric token, so
100% could trigger a FormatException before rendering.

At the same time, the repository already has conformance coverage that treats other percentage
opacity values such as opacity="0.1%" as invalid input. A broad “parse all percentages” fix
would stop the exception, but it would also regress those existing renderer expectations.

What Changed

1. Narrow the parser fix to the safe case

File:

  • src/Svg.Custom/Compatibility/SvgElementFactory.cs

The compatibility loader now special-cases percentage opacity values as follows:

  • 100% is normalized to 1
  • any other percentage opacity token is treated as an invalid declaration and ignored

That avoids the load-time exception for the reported SVG without changing the rendered result for
the safe 100% case, and it preserves the prior behavior for invalid percentage opacity input.

The change still applies only to opacity-related properties:

  • opacity
  • fill-opacity
  • stroke-opacity
  • stop-opacity
  • flood-opacity

It also keeps the existing compatibility handling for:

  • opacity="undefined"
  • stop-opacity="inherit"

2. Use newer span APIs where the target framework supports them

File:

  • src/Svg.Custom/Compatibility/SvgElementFactory.cs

The helper path now uses framework span APIs on newer target frameworks and keeps a local fallback
for netstandard2.0:

  • ReadOnlySpan<char>.Trim() on newer TFMs
  • manual span trimming on NETSTANDARD20
  • span-based float.TryParse(...) where available

This keeps the code allocation-lean on modern targets without breaking the existing target matrix.

3. Update regression coverage to match the intended semantics

File:

  • tests/Svg.Skia.UnitTests/SvgDocumentCompatibilityLoaderTests.cs

The parser tests now verify:

  • opacity="100%" loads and resolves to the default opacity value (1)
  • invalid percentage opacity inline styles are ignored instead of throwing

This aligns the tests with the actual compatibility policy instead of asserting broad percentage
parsing support.

4. Keep the public Avalonia regression

File:

  • tests/Svg.Controls.Skia.Avalonia.UnitTests/SvgSourceTests.cs

The Avalonia regression test remains in place to cover the original user-visible entry point:

  • SvgSource.LoadFromSvg(...) with the reported opacity="100%" SVG no longer throws

Commit Breakdown

  1. Normalize percentage opacity attributes

    • initial parser fix and regression coverage
  2. Add SvgSource opacity regression test

    • Avalonia integration-path regression
  3. Use spans in opacity normalization

    • span-based helper path on newer targets
  4. Ignore invalid percentage opacity values

    • preserve invalid-value semantics while keeping the crash fix

Validation

Commands run:

dotnet format Svg.Skia.slnx --no-restore
dotnet build Svg.Skia.slnx -c Release
dotnet test Svg.Skia.slnx -c Release

Focused checks also run during investigation:

dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~resvgTests.a_opacity&DisplayName~a-opacity-005"
dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SvgDocumentCompatibilityLoaderTests"
dotnet test tests/Svg.Controls.Skia.Avalonia.UnitTests/Svg.Controls.Skia.Avalonia.UnitTests.csproj -f net10.0 -c Release --filter "FullyQualifiedName~SvgSourceTests.LoadFromSvg_WithPercentageOpacity_DoesNotThrow"

Results:

  • full solution build: passed
  • full solution test suite: passed
  • focused resvg regression (a-opacity-005): passed

Risk Assessment

Risk is low because:

  • the parser change is limited to opacity-related properties,
  • the fix only normalizes the safe 100% case,
  • other invalid percentage opacity values are ignored instead of being reinterpreted,
  • both parser-level and Avalonia integration regressions are covered,
  • the full solution test suite passed after the change.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30105c8224

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Svg.Custom/Compatibility/SvgElementFactory.cs Outdated
…-parsing

# Conflicts:
#	src/Svg.Custom/Compatibility/SvgElementFactory.cs
#	tests/Svg.Controls.Skia.Avalonia.UnitTests/SvgSourceTests.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant