LibWeb/LibGfx: Handle differences in overflow-x and overflow-y correctly

[PrestonLTaylor]

This pr allows for handling of overflow-x and overflow-y being different correctly.

This would cause issues if only one dimension of overflow was hidden and could cause content to be rendered when it should be hidden.

Example code:

<style>
  .x {
    width: 0rem;
    overflow-x: hidden;
  }

  .y {
    height: 0rem;
    overflow-y: hidden;
  }
</style>
<div class="x">
  <p>Should be hidden!</p>
</div>
<div class="y">
  <p>Should be hidden!</p>
</div>

Before:

After:

[BuggieBot]

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

[MacDue]

Looks like UBSAN is not happy with this changes:

/home/vsts/work/1/s/Meta/Lagom/../../AK/DistinctNumeric.h:268:23: runtime error: signed integer overflow: -2147483648 + -50 cannot be represented in type 'int'
    #0 0x7ff546379b4c in AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>::operator+=(AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> const&) /home/vsts/work/1/s/Meta/Lagom/../../AK/DistinctNumeric.h:268
    #1 0x7ff546379b4c in Gfx::Point<AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> >::translate_by(AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>, AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>) /home/vsts/work/1/s/Meta/Lagom/../../Userland/Libraries/LibGfx/Point.h:56
    #2 0x7ff546379b4c in Gfx::Point<AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> >::translated(AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>, AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>) const /home/vsts/work/1/s/Meta/Lagom/../../Userland/Libraries/LibGfx/Point.h:83
    #3 0x7ff546379b4c in Web::Painting::BorderRadiusCornerClipper::create(Web::PaintContext&, Gfx::Rect<AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> > const&, Web::Painting::BorderRadiiData const&, Web::Painting::CornerClip, Web::Painting::BorderRadiusCornerClipper::UseCachedBitmap) /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp:46
    #4 0x7ff546523a88 in Web::Painting::PaintableBox::apply_clip_overflow_rect(Web::PaintContext&, Web::Painting::PaintPhase) const /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp:380
    #5 0x7ff5465fd2e9 in Web::Painting::StackingContext::paint_descendants(Web::PaintContext&, Web::Layout::Node const&, Web::Painting::StackingContext::StackingContextPaintPhase) const /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/StackingContext.cpp:79
    #6 0x7ff54663ca05 in operator() /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/StackingContext.cpp:205

[PrestonLTaylor]

Looks like UBSAN is not happy with this changes:

/home/vsts/work/1/s/Meta/Lagom/../../AK/DistinctNumeric.h:268:23: runtime error: signed integer overflow: -2147483648 + -50 cannot be represented in type 'int'
    #0 0x7ff546379b4c in AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>::operator+=(AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> const&) /home/vsts/work/1/s/Meta/Lagom/../../AK/DistinctNumeric.h:268
    #1 0x7ff546379b4c in Gfx::Point<AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> >::translate_by(AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>, AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>) /home/vsts/work/1/s/Meta/Lagom/../../Userland/Libraries/LibGfx/Point.h:56
    #2 0x7ff546379b4c in Gfx::Point<AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> >::translated(AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>, AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment>) const /home/vsts/work/1/s/Meta/Lagom/../../Userland/Libraries/LibGfx/Point.h:83
    #3 0x7ff546379b4c in Web::Painting::BorderRadiusCornerClipper::create(Web::PaintContext&, Gfx::Rect<AK::DistinctNumeric<int, Web::__DevicePixels_tag, AK::DistinctNumericFeature::Arithmetic, AK::DistinctNumericFeature::CastToUnderlying, AK::DistinctNumericFeature::Comparison, AK::DistinctNumericFeature::Increment> > const&, Web::Painting::BorderRadiiData const&, Web::Painting::CornerClip, Web::Painting::BorderRadiusCornerClipper::UseCachedBitmap) /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/BorderRadiusCornerClipper.cpp:46
    #4 0x7ff546523a88 in Web::Painting::PaintableBox::apply_clip_overflow_rect(Web::PaintContext&, Web::Painting::PaintPhase) const /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp:380
    #5 0x7ff5465fd2e9 in Web::Painting::StackingContext::paint_descendants(Web::PaintContext&, Web::Layout::Node const&, Web::Painting::StackingContext::StackingContextPaintPhase) const /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/StackingContext.cpp:79
    #6 0x7ff54663ca05 in operator() /home/vsts/work/1/s/Userland/Libraries/LibWeb/Painting/StackingContext.cpp:205

Seems like it was with this bit of code m_clip_rect = CSSPixelRect { 0, padding_box.y(), AK::NumericLimits<int>::max(), padding_box.height() };

Specifically with the NumericLimits::max() being too large when converting into a device rect.

I've changed the type to short so it shouldn't overflow, but is there a better way of signalling if a rect component should be ignored?

[MacDue]

One option would be not to use a Gfx::Rect and make something like this (below) when computing the overflow clip, since I guess it's not really a rectangle.

template<T>
struct ClipRange { 
    T min, max;
};

struct Clip {
   Optional<ClipRange<CSSPixels>> x_clip;
   Optional<ClipRange<CSSPixels>> y_clip;
};

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

[stale[bot]]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!