Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes geometry render bounds when curves are present #16756

Merged
merged 4 commits into from
Sep 12, 2024

Conversation

MrJul
Copy link
Member

@MrJul MrJul commented Aug 21, 2024

What does the pull request do?

This PR fixes the render bounds of geometries containing curves.

What is the current behavior?

The render bounds are too large, containing the curve control points.

Example:
before

At the top is DrawingImage with incorrect bounds, having the height almost twice as high as necessary.
At the bottom is a Path with correct bounds (Path has its own bounds calculation).

Show code
<StackPanel Orientation="Vertical" Spacing="20" Margin="20">

  <Border HorizontalAlignment="Left" Background="Blue">
    <Image Width="128">
      <DrawingImage>
        <DrawingImage.Drawing>
          <GeometryDrawing Brush="Yellow" Geometry="{StaticResource Geometry}" />
        </DrawingImage.Drawing>
      </DrawingImage>
    </Image>
  </Border>

  <Border HorizontalAlignment="Left" Background="Blue">
    <Path Width="128" Fill="Yellow" Data="{StaticResource Geometry}" />
  </Border>

</StackPanel>

Geometry:

<Geometry x:Key="Geometry">M0,0 A128,128 0 0 0 128,0</Geometry>

What is the updated/expected behavior with this PR?

The bounds are correctly computed.

Result:
after

How was the solution implemented (if it's not obvious)?

SKPath.TightBounds is used instead of SKPath.Bounds.
(These were the only two usages of Bounds, other types using SKPath were already using TightBounds.)

A unit test has been added.
Note that an existing unit test has been modified, because it used round line caps, which caused the same problem.

@MrJul
Copy link
Member Author

MrJul commented Aug 21, 2024

The new test was passing on Windows but failing on Linux and macOS due to a floating point rounding error at the 6th decimal (17.14875030517578 vs 17.148752212524414).

The tests were already using a tolerance of 0.001, but still checked for a perfect Equals match right after. This has been changed to use xUnit's Assert.Equal(x, y, tolerance) instead.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0051418-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald Gillibald added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Sep 12, 2024
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0051777-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added this pull request to the merge queue Sep 12, 2024
Merged via the queue into AvaloniaUI:master with commit 932f543 Sep 12, 2024
11 checks passed
@MrJul MrJul deleted the fixes/geometry-render-bounds branch September 19, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-rendering backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants