Improve and optimize non-uniform Borders. - #124417

Sorry, @bernaferrari but to be honest, I'm not a fan. The "old" way is boring, but predictable and unsurprising. The "new" way seems pretty surprising to me, and not at all clear why the magenta spans the width and the red doesn't span the height.

EDIT: Ahh, I see you've updated the definitions of old and new on me. :-) OK, I'll take a look, but I think it would need to be less surprising than it currently is.

For borderRadius there is no old way, it just crashes.

This is the alternate to crashing:

It is possible for the current implementation to overlap with each other (I just don't know exactly how). I am proposing a muuuuch more agressive overlap (where possible) while respecting the original order.

Another possibility is: still crash when top+left with borderRadius, but don't crash when top+bottom or left+right with borderRadius.

I just commited this possibility, check if you are happier (if you are, I just need to re-add some crashing tests).

Why do they overlap at all ? Can't they be in diagonal like they are without border radius ? The border radius would just round the edges. Like this:

They could, by clipping (slow). But I limited the behavior to only work on top, left, bottom, right or top/bottom, right/left (but these are less important).

Also, your solution wouldn't work for circular shape :(

I think if this PR addressed top/left/right/bottom would be enough already. Because it is a very simple scenario (Border.only) and the current solution fails on that, since it defaults everything else to none.

I changed from addressing "all cases" to addressing Border.only and Border.symmetrical, but I can change to support Border.only as well. What do you think?

My biggest motivator is that Border.only and Border.symmetric doesn't work because BorderSide.none uses BorderSide.color = Colors.black by default, so I think...
a) Allow painting if there is a single color and everything else is none.
b) Allow painting if there are two opposite colors (paints twice).

Or a combination of a+b.

What do you think @gspencergoog?

By the principle of least surprise, I agree with @cedvdb. Even if it's slower, it's much less surprising. For one thing, the lerp between no border radius and border radius would need to be smooth, and I don't see how you can do that with your design, at least not in any pleasing way.

What about supporting Border.only with a single visible color (and maybe Border.symmetrical), ignoring Border.only with multiple colors for now?

What about supporting Border.only with a single visible color (and maybe Border.symmetrical), ignoring Border.only with multiple colors for now?

That might work. What would that look like?

When we merged it with borderRadius, we forgot the "default" of Border.only and Border.symmetrical is black + invisible by default. So I think we could support non-uniform borderStyle (BorderStyle.none).

Border.only(top: ...) will fail with borderRadius because left/right/bottom are invisible with different colors.

I think option B seems more reasonable. Option A seems like people will just wonder why the limitation exists. I mean, they'll wonder with either one, but I think it's easier to see the logic with Option B.

Up to this point, it wasn't supposed to trigger Google Testing. Now it might. Not using paintBorder (when possible) is much faster than using it.

Goldens will also trigger. It is the path -> drrect difference.

I painted 100k borders on each test. This PR is ~15-50% faster.

00:05 +0: Divider control test                                                                                                                            
BoxBorder._paintNonUniformBorder: 80
BoxBorder.paintBorder: 101
00:05 +1: Divider custom thickness                                                                                                                        
BoxBorder._paintNonUniformBorder: 70
BoxBorder.paintBorder: 107
00:05 +2: Horizontal divider custom indentation                                                                                                           
BoxBorder._paintNonUniformBorder: 60
BoxBorder.paintBorder: 88
BoxBorder._paintNonUniformBorder: 61
BoxBorder.paintBorder: 93
BoxBorder._paintNonUniformBorder: 61
BoxBorder.paintBorder: 87
00:06 +3: Vertical Divider Test                                                                                                                           
BoxBorder._paintNonUniformBorder: 57
BoxBorder.paintBorder: 87
00:06 +4: Divider custom thickness                                                                                                                        
BoxBorder._paintNonUniformBorder: 62
BoxBorder.paintBorder: 95
00:06 +5: Vertical Divider Test 2                                                                                                                         
BoxBorder._paintNonUniformBorder: 57
BoxBorder.paintBorder: 91
BoxBorder._paintNonUniformBorder: 383
BoxBorder.paintBorder: 434
00:07 +6: Vertical divider custom indentation                                                                                                             
BoxBorder._paintNonUniformBorder: 65
BoxBorder.paintBorder: 89
BoxBorder._paintNonUniformBorder: 65
BoxBorder.paintBorder: 90
BoxBorder._paintNonUniformBorder: 68
BoxBorder.paintBorder: 91
00:08 +8: All tests passed!                                                                                                                              

Nice! Let's see how bad the Google test fallout is.