Re-Land "Refactor StrokeAlign to allow double values." (#108339) #110234
Conversation
flutter-dashboard
bot
added
d: api docs
| if (side.width == 0.0) { | ||
| canvas.drawRRect(borderRadius.resolve(textDirection).toRRect(rect), side.toPaint()); | ||
| } else { | ||
| final Paint paint = Paint() | ||
| ..color = side.color; | ||
| final RRect borderRect = borderRadius.resolve(textDirection).toRRect(rect); | ||
| final RRect inner = borderRect.deflate(side.strokeInset); | ||
| final RRect outer = borderRect.inflate(side.strokeOutset); | ||
| canvas.drawDRRect(outer, inner, paint); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Is canvas.drawRRect better than break;? I'm curious if there was a difference.
Your break; in 139 is 'unecessary' (if you add, you might want to add in 130, after drawRRect, and on the other borders. But break = nothing so it doesn't matter.
There was a problem hiding this comment.
I realize that it doesn't do anything, but it provides consistency with the other case statement, and if a new case is added, the person adding it won't need to add a break.
There was a problem hiding this comment.
Oh, sure. Feel free to add to the other borders if you feel it is relevant, I think only the Continous or Beveled have it. The other ones don't.
|
Thanks for your persistence on this! |
Isn't this the previous sample that we made and got merged/reverted/re-merged/re-reverted? I'm kind of confused. But everything is ok. |
Yeah, could be. :-). I just noticed it wasn't linked in the docs, and that the files were sitting on my disk, so I added them. |
| adjustedRect = borderRect; | ||
| break; | ||
| } | ||
| final RRect adjustedRect = borderRect.deflate(ui.lerpDouble(side.width, 0, side.strokeAlign)!); |
There was a problem hiding this comment.
I think you are using an older version. The one that was merged didn't have this. We replace the lerp with strokeInset.
There was a problem hiding this comment.
Oooh, thanks for catching that, I'll make sure I'm reverting the right thing.
There was a problem hiding this comment.
Wait... I'm not seeing the same thing you are.
In both the originally landed PR, and here, the line is:
final RRect adjustedRect = borderRect.deflate(ui.lerpDouble(side.width, 0, side.strokeAlign)!);
You're looking at line 100 on the right, not line 296.
The real question is: should that be changed? It seems to work as-is.
There was a problem hiding this comment.
You're right, though, that the example is the same as the original, I just linked it into the docs, and renamed it to match the naming convention.
There was a problem hiding this comment.
Ok, forgive me. I got confused and looked in the wrong place. It was late at night 😅.
gspencergoog
force-pushed
the
stroke_align
branch
from
52cca65
to
ff926fc
Compare
This reverts commit 957a8da to try again at landing it.
gspencergoog
force-pushed
the
stroke_align
branch
from
ff926fc
to
091ee75
Compare
|
I just double-checked everything, everything looks good to me. |
gspencergoog
added
the
autosubmit
| /// - [inside] provides padding with full [BorderSide.width]. | ||
| /// - [center] provides padding with half [BorderSide.width]. | ||
| /// - [outside] provides zero padding, as stroke is drawn entirely outside. | ||
| enum StrokeAlign { |
There was a problem hiding this comment.
why is removing this not a breaking change? the code I wrote a few months ago became red w/o any pointers on why or how to fix it. i had to go into the flutter code, see the last changing commit and go through the PR to understand the reason and cause of the breakage. am i missing something?
There was a problem hiding this comment.
AFAIK it is complicated, there is no easy way to keep that without making a weird class that acts as a shortcut for doubles, even though I tried, it was weird and non-standard, so BorderSide.strokeAlignInside became the default (you can read the discussion in the original PR). The "part 1" and "part 2" commits were like 2 months apart and strokeAlign was still really new when it was modified, but due to how Flutter stable works, they ended up being like 6 months apart.
There was a problem hiding this comment.
hmm that makes sense. if only dart supported union types natively.
There was a problem hiding this comment.
Me too, but we can only hope they prioritize that for 2024.
Description
This re-lands #108339 with some updates that keep the rendering the same as it was before so that they don't break golden tests. There are still some acceptable very small (one grey level) rounding differences in
BoxBorderrendering, but everything else is closer to where it should have been, especially the corners on zero-width rounded rectangles, and zero-radius corners onContinuousRectangleBorders.I also renamed the sample, and linked it to the
BorderSide.strokeAlignmember docs.The first commit is the original re-land, and the subsequent commits are my changes.
Related Issue: #106362
cc @bernaferrari
internal reference: b/243217872
G3Fix: cl/469882656