Improve and optimize non-uniform Borders.

[bernaferrari]

Using the same priority order as a Border without borderRadius, it is possible to draw them on top of each other. This is better than the current behavior (crash!) and would work well for a "one color on top, another on bottom" scenario.

With this, if approved, we move the current number of possible exceptions from 4 to 1 (BoxShape.circle + borderRadius).

It is kind of odd how borderRadius.zero to borderRadius != BorderRadius.zero change, but I think it is better than crashing. Alternatively, we just remove the "original function" and see if any goldens are affected.

Another one for @gspencergoog. If this works, we could make the paint method public and re-use in the InputBorder PR (if that's also approved). Single line fix.

[gspencergoog]

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.

[bernaferrari]

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

[bernaferrari]

This is the alternate to crashing:

[bernaferrari]

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.

[bernaferrari]

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).

[cedvdb]

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:

[bernaferrari]

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?

[bernaferrari]

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?

[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.

[bernaferrari]

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

[gspencergoog]

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?

[bernaferrari]

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.

[gspencergoog]

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.

[bernaferrari]

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!                                                                                                                              

[gspencergoog]

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

[tirth-multipl]

this new assert broke app-bar in my app after updating to 3.16.

[tirth-multipl]

@Hixie @bernaferrari is it a good idea to mention this change in breaking points page for 3.16?

[bernaferrari]

Can you show the code? It isn't supposed to break anything because the behavior was wrong

[tirth-multipl]

made a reproducible sample: https://dartpad.dev/?id=413770c667697ac3472fc03d12d4fe54

Issue: "FAQs" widget is not shown because of the new assert.

---

either setting the border-radius to zero or removing the BorderSide's with 0 width, fixes the problem. not sure why we were using border-side 0 in our code but code was in app-bar hence I noticed it when it glitched after stable update.

[bernaferrari]

Yeah, but the 0 as hidden is a bug, because flutter uses that for hairline border (I wish it were different but I can't change it because visual density depends on context), so instead of fixing to show it (which would be a bug for you), we fixed to assert to prevent problems in the future

[kekland]

Same here. Not sure what was wrong with hairline borders, but we use them pretty extensively in our app and now they aren't visible anymore after updating to the latest Flutter version. Could you clarify what was the issue with drawing hairline borders with rounded corners?

[bernaferrari]

This is an inconsistent behavior, I just updated to be consistent. The issue is that for any other corner it draws hairline on width = 0 and you can't draw it on non uniform, so I added an assert.