community icon indicating copy to clipboard operation
community copied to clipboard

Line/SmoothLine: Fixes rendering issues related to corner radius and updates its order (`rounded_rectangle`) + add getter methods for `rounded_rectangle`, `rectangle`, `ellipse`, `circle`.

Open DexerBR opened this issue 3 years ago • 2 comments

This PR is intended to solve some common problems of the rounded_rectangle used with Line/SmoothLine:

  • Improve segment control to avoid unwanted shapes if any corner radius = 0 or too larger. The minimum computed radius will be 1, and the maximum will be the smallest dimension between width and height. This makes it possible to freely switch shapes between rectangle and circle while using rounded_rectangle without implementing workarounds to avoid the issue.

  • Updates the default resolution to 45. The current value results in slightly imprecise shapes.

  • Prevents rendering issues if width or height is less than 2px when using SmoothLine.

  • Corner_radius order changed to match RoundedRectangle radius property (clockwise). There is no standardization between the radius of rounded_rectangle and RoundedRectangle. Previously rounded_rectangle received bottom-left, bottom-right, top-right, top-left. With this PR, it defaults to RoundedRctangle: top-left, top-right, bottom-right, bottom-left (clockwise).

Also includes:

  • Unittests for rounded_rectangle (Line/SmoothLine).
  • Getter methods to rounded_rectangle, rectangle, ellipse and circle.

Maintainer merge checklist

  • [x] Title is descriptive/clear for inclusion in release notes.
  • [x] Applied a Component: xxx label.
  • [x] Applied the api-deprecation or api-break label.
  • [x] Applied the release-highlight label to be highlighted in release notes.
  • [x] Added to the milestone version it was merged into.
  • [x] Unittests are included in PR.
  • [x] Properly documented, including versionadded, versionchanged as needed.

DexerBR avatar Apr 30 '22 02:04 DexerBR

Considering this changes current behaviour, We need to document the change, plus unit tests checking the corner cases would be nice.

akshayaurora avatar Jun 20 '22 05:06 akshayaurora

A useful code to test Line/SmoothLine with rounded_rectangle.

from kivy.app import App
from kivy.uix.floatlayout import FloatLayout
from kivy.lang import Builder

Builder.load_string("""
<LineTest>:
    w: 100
    h: 100
    r1: 1
    r2: 1
    r3: 1
    r4: 1
    segs: 45
    _width: 1
    ov: 2
    canvas:

        Color:
            rgba: 0, 0, 1, 1
        SmoothLine:
            width: root._width
            overdraw_width: root.ov
            rounded_rectangle: (50, 300, root.w * 2, root.h, root.r1, root.r2, root.r3, root.r4, root.segs)

        Color:
            rgba: 1, 0, 0, 1
        SmoothLine:
            width: root._width
            overdraw_width: root.ov
            rounded_rectangle: (100, 350, root.w, root.h * 2, root.r1, root.r2, root.r3, root.r4, root.segs)

        Color:
            rgba: 0, 1, 0, 1
        SmoothLine:
            width: root._width
            overdraw_width: root.ov
            rounded_rectangle: (150, 425, root.w, root.h, root.r1, root.r2, root.r3, root.r4, root.segs)

        Color:
            rgba: 1, 1, 1, 1
        Line:
            width: root._width
            rounded_rectangle: (275, 450, root.w, root.h, root.r1, root.r2, root.r3, root.r4, root.segs)

    BoxLayout:
        orientation: "vertical"
        size_hint_y: None
        height: self.minimum_height
        Label:
            text: f"line_width:{root._width:.2f}, ov:{root.ov:.2f}, w:{root.w:.2f}, h:{root.h:.2f}, r1:{root.r1:.2f}, r2:{root.r2:.2f}, r3:{root.r3:.2f}, r4:{root.r4:.2f}, segs:{root.segs:.2f}"
        Slider:
            size_hint: 1, None
            height: 30
            min: 1
            max: 50
            on_value:
                root._width = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: 1
            max: 50
            on_value:
                root.ov = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: -50
            max: 500
            value: 0
            step: 1
            on_value:
                root.w = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: -50
            max: 500
            value: 0
            step: 1
            on_value:
                root.h = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: 0
            max: 500
            on_value:
                root.r1 = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: 0
            max: 500
            on_value:
                root.r2 = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: 0
            max: 500
            on_value:
                root.r3 = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: 0
            max: 500
            on_value:
                root.r4 = self.value
        Slider:
            size_hint: 1, None
            height: 30
            min: 0
            max: 500
            on_value:
                root.segs = int(self.value)
""")


class LineTest(FloatLayout):
    ...


class TestApp(App):
    def build(self):
        return LineTest()

TestApp().run()

DexerBR avatar Jul 11 '22 20:07 DexerBR

Hi @tshirtman ! Is there anything else that needs to be addressed before merging this one?

Thanks in advance!

DexerBR avatar Nov 05 '22 12:11 DexerBR

Did a quick test, and also LGTM.

Agree with @tshirtman :

if there are important issues, they will surely be found before release.

misl6 avatar Mar 12 '23 15:03 misl6