Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Jul 7, 2023

CP issue: flutter/flutter#130149

Fixes flutter/flutter#130084

If a display list is drawn into another display list and the child display list establishes a small clip, subsequent drawing operations are discarded when really they should not be.

The test is expected to render both a blue and a red square; before the fix it renders only the blue square since the red square is incorrectly clipped out.

See also dnfield/flutter_svg#938

Fixes flutter/flutter#130084

If a display list is drawn into another display list and the child display list establishes a small clip, subsequent drawing operations are discarded when really they should not be.

The test is expected to render both a blue and a red square; before the fix it renders only the blue square since the red square is incorrectly clipped out.

See also dnfield/flutter_svg#938
@flutter-dashboard
Copy link

This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter.

Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed.

@CaseyHillers
Copy link
Contributor

FYI presubmit is failing on

FAILED: obj/flutter/impeller/display_list/display_list_unittests.display_list_unittests.o 
/b/s/w/ir/cache/goma/client/gomacc ../../buildtools/linux-x64/clang/bin/clang++ -MD -MF obj/flutter/impeller/display_list/display_list_unittests.display_list_unittests.o.d -DUSE_OPENSSL=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DIMPELLER_SUPPORTS_RENDERING=1 -DIMPELLER_ENABLE_OPENGLES=1 -DIMPELLER_ENABLE_VULKAN=1 -DSK_TYPEFACE_FACTORY_FREETYPE -DSK_GL -DSK_VULKAN -DSK_CODEC_DECODES_JPEG -DSK_ENCODE_JPEG -DSK_CODEC_DECODES_PNG -DSK_ENCODE_PNG -DSK_CODEC_DECODES_WEBP -DSK_ENCODE_WEBP -DSK_HAS_WUFFS_LIBRARY -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 -DDART_LEGACY_API=\[\[deprecated\]\] -DFLUTTER_RUNTIME_MODE=3 -DFLUTTER_RELEASE=1 -DSK_R32_SHIFT=16 -DSK_ENABLE_DUMP_GPU -DSK_DISABLE_AAA -DSK_LEGACY_IGNORE_DRAW_VERTICES_BLEND_WITH_NO_SHADER -DSK_DISABLE_LEGACY_SHADERCONTEXT -DSK_DISABLE_LOWP_RASTER_PIPELINE -DSK_FORCE_RASTER_PIPELINE_BLITTER -DSK_METAL_WAIT_UNTIL_SCHEDULED -DSK_DISABLE_EFFECT_DESERIALIZATION -DSK_ENABLE_SKSL -DSK_ENABLE_PRECOMPILE -DSK_GANESH -DSK_USE_PERFETTO -DSK_LEGACY_LAYER_BOUNDS_EXPANSION -I../.. -Igen -I../../third_party/libcxx/include -I../../third_party/libcxxabi/include -I../../build/secondary/third_party/libcxx/config -I../../flutter -Igen/flutter -I../../third_party/flatbuffers/include -I../../third_party/skia -I../../third_party/glfw/include -I../../third_party/glfw/include/GLFW -I../../third_party/imgui -I../../third_party/googletest/googlemock/include -I../../third_party/googletest/googletest/include -Wthread-safety-analysis -fno-strict-aliasing -fstack-protector --param=ssp-buffer-size=4 -m64 -march=x86-64 -fPIC -pipe -pthread -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-c99-designator -Wno-deprecated-copy -Wno-psabi -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -fvisibility=hidden --sysroot=/b/s/w/ir/cache/builder/src/build/linux/debian_sid_amd64-sysroot -Wstring-conversion -Wnewline-eof -O2 -fno-ident -fdata-sections -ffunction-sections -g0 -Wno-newline-eof -fvisibility-inlines-hidden -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions -Wno-inconsistent-missing-override   -c ../../flutter/impeller/display_list/display_list_unittests.cc -o obj/flutter/impeller/display_list/display_list_unittests.display_list_unittests.o
../../flutter/impeller/display_list/display_list_unittests.cc:50:15: error: too few arguments to function call, expected 3, have 1; did you mean 'flutter::DlCanvas::ClipRect'?
  sub_builder.ClipRect(SkRect::MakeXYWH(0, 0, 24, 24));
              ^~~~~~~~
              flutter::DlCanvas::ClipRect
../../flutter/display_list/dl_canvas.h:100:16: note: 'flutter::DlCanvas::ClipRect' declared here
  virtual void ClipRect(const SkRect& rect,
               ^
1 error generated.

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm after fixing the merge issue.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 7, 2023

An intermediate commit made the arguments optional, providing the correct values for them now in the test.

@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 10, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 10, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 10, 2023

auto label is removed for flutter/engine, pr: 43469, due to - The status or check suite Windows Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Windows windows_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Windows Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac Host clang-tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux Host clang-tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux Benchmarks (no-upload) has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield
Copy link
Contributor Author

dnfield commented Jul 10, 2023

missed a namespace qualifier...

@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 10, 2023
@auto-submit auto-submit bot merged commit f9bf65c into flutter:flutter-3.10-candidate.1 Jul 10, 2023
auto-submit bot pushed a commit that referenced this pull request Jul 11, 2023
)

Cherry pick the same pick as for 3.10 (#43469 and flutter/flutter#130149).

Fixes flutter/flutter#130084

If a display list is drawn into another display list and the child display list establishes a small clip, subsequent drawing operations are discarded when really they should not be.

The test is expected to render both a blue and a red square; before the fix it renders only the blue square since the red square is incorrectly clipped out.

See also dnfield/flutter_svg#938
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants