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

Conversation

@LongCatIsLooong
Copy link
Contributor

This reverts commit 6191270.

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. You must list at least one issue.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.
  • The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla google-cla bot added the cla: yes label Feb 26, 2021
bitmask |= (forceStrutHeight ? 1 : 0) << 8;
}

data.setInt8(0, bitmask);
Copy link
Member

Choose a reason for hiding this comment

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

This will need to be expanded to setInt16 now that there are more than 8 flag bits.

The initialization of byteCount at the beginning of _encodeStrut will also change to 2 to match the width of the bit mask.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

squeezed forceStrutHeight into 1 bit by treating null as false.

lib/ui/text.dart Outdated
byteCount += 4;
if (leadingDistribution != null) {
bitmask |= 1 << 5;
data.setInt8(byteCount, leadingDistribution.index);
Copy link
Member

Choose a reason for hiding this comment

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

This does not match the logic in decodeStrut.

decodeStrut assumes that all the float fields are placed after the byte fields in the buffer. But _encodeStrut is placing the leadingDistribution byte after the height float.

Also, the byteCount += 4 here does not match the setInt8, and it could exceed the size of the ByteData buffer.

I'd recommend moving the encoding of this field up so that it precedes the float fields.

@LongCatIsLooong
Copy link
Contributor Author

LongCatIsLooong commented Feb 27, 2021

Diff: 3765e9a. Still have some Scuba failures but line spacing looks correct now.

@LongCatIsLooong LongCatIsLooong marked this pull request as ready for review February 27, 2021 04:33
@LongCatIsLooong LongCatIsLooong added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Mar 1, 2021
@fluttergithubbot fluttergithubbot merged commit ef8353b into flutter:master Mar 1, 2021
@LongCatIsLooong LongCatIsLooong deleted the reland-24665 branch March 1, 2021 22:59
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 2, 2021
pull bot pushed a commit to Mu-L/skia that referenced this pull request Apr 16, 2021
Change-Id: I5627cb163400880992db622bb887093da92d1cd5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394969
Commit-Queue: Julia Lavrova <[email protected]>
Reviewed-by: Julia Lavrova <[email protected]>
skia-flutter-autoroll added a commit to skia-flutter-autoroll/engine that referenced this pull request Apr 16, 2021
https://skia.googlesource.com/skia.git/+log/e2b457ad5fab..624a529fbd01

2021-04-16 [email protected] Added an API for creating RuntimeEffects using the SkSL DSL.
2021-04-16 [email protected] Generalize the squircle runtime effect to draw "super rounded rects"
2021-04-16 [email protected] Viewer: apply same transform as slide to dimensions display.
2021-04-16 [email protected] Lower GPU budget to 16MB on some desktop bots
2021-04-16 [email protected] Add halfLeading to TextStyle and StrutStyle: flutter#24668
2021-04-16 [email protected] Fix android framework build in GrVkOpsRenderPass.
2021-04-16 [email protected] Incorporate top-level DSL blocks into fragment processors.
2021-04-16 [email protected] Fix gm dimensions
2021-04-16 [email protected] Use GrVkFramebuffer throughout GrVkOpsRenderPass instead of GrVkRT.
2021-04-16 [email protected] Fix gpuResourceCacheLimit implementation
2021-04-16 [email protected] Expose experimental iterator on SkParagraph

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/skia-flutter-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/master/autoroll/README.md
JiaLiangAl pushed a commit to JiaLiangAl/chromium that referenced this pull request Apr 17, 2021
https://skia.googlesource.com/skia.git/+log/da34d099fcab..624a529fbd01

2021-04-16 [email protected] Added an API for creating RuntimeEffects using the SkSL DSL.
2021-04-16 [email protected] Generalize the squircle runtime effect to draw "super rounded rects"
2021-04-16 [email protected] Viewer: apply same transform as slide to dimensions display.
2021-04-16 [email protected] Lower GPU budget to 16MB on some desktop bots
2021-04-16 [email protected] Add halfLeading to TextStyle and StrutStyle: flutter/engine#24668
2021-04-16 [email protected] Fix android framework build in GrVkOpsRenderPass.
2021-04-16 [email protected] Incorporate top-level DSL blocks into fragment processors.

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/skia-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/master/autoroll/README.md

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Cq-Do-Not-Cancel-Tryjobs: true
Bug: None
Tbr: [email protected]
Change-Id: Ia1da5744a7ce10c5d20dfa08238ffe909ec81950
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2830208
Commit-Queue: chromium-autoroll <[email protected]>
Bot-Commit: chromium-autoroll <[email protected]>
Cr-Commit-Position: refs/heads/master@{#873514}
romanzes pushed a commit to romanzes/skia that referenced this pull request Nov 22, 2021
Change-Id: I5627cb163400880992db622bb887093da92d1cd5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394969
Commit-Queue: Julia Lavrova <[email protected]>
Reviewed-by: Julia Lavrova <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants