GraphiteEditor / Graphite

2D vector & raster editor that melds traditional layers & tools with a modern node-based, non-destructive, procedural workflow.
https://graphite.rs
Apache License 2.0
7.77k stars 408 forks source link

Add corner rounding to the Rectangle node #1648

Closed skoriop closed 5 months ago

skoriop commented 7 months ago

This PR will add a border radius property that will allow the creation of rounded rectangles.

Closes #1642

Keavon commented 7 months ago

!build

github-actions[bot] commented 7 months ago
📦 Build Complete for 6614b3b363ad2f6c209fb39b26d78cc3f61765f6
https://eecc9b29.graphite.pages.dev
Keavon commented 7 months ago

Looks good. Can you also please add a checkbox parameter, that's enabled by default, called "Clamped" which prevents going below 0 and above half the adjacent dimension(s)? So a 50x100 rectangle, if clamped, would have a corner with a radius of 25x50 (similar to the behavior of CSS).

Keavon commented 7 months ago

And could you please add the all-corners/individual-corners toggle between setting a scalar and vector4 value? With the methodology discussed on Discord. Thanks.

I also rebased this, so be sure to pull.

Keavon commented 6 months ago

I'll mark this as a draft until the per-corner feature is ready. Please let us know on Discord if you need any help making the generic inputs (scalar or vec4) work. If you prefer, instead, to merge this as-is and add the per-corner feature in a later PR, that's also acceptable (just mention your preference here and we can merge it sooner). Please ping me once you're ready for review again.

otdavies commented 6 months ago

!build

github-actions[bot] commented 6 months ago
📦 Build Complete for 709190a177c09dacf4f73834525ac1797d0f18f6
https://87585acf.graphite.pages.dev
Keavon commented 6 months ago

Can you please add the logic so, when switching from Uniform to Individual, it sets the four values to the value that existed; and when switching from Individual to Uniform, it sets the single value to the first of the four values?

otdavies commented 6 months ago

Crash found, when you have clamp enabled negative scale causes the editor to crash: chrome_Hm06xgHHHs

skoriop commented 6 months ago

Can you please add the logic so, when switching from Uniform to Individual, it sets the four values to the value that existed; and when switching from Individual to Uniform, it sets the single value to the first of the four values?

Done, along with fixing the negative scale crash

Keavon commented 6 months ago

!build

github-actions[bot] commented 6 months ago
📦 Build Complete for daa90553481d83615677715ec8fe8ded52f99d07
https://2ba05e9c.graphite.pages.dev
Keavon commented 6 months ago

One last little issue, if you switch back from Individual to Uniform, it does now display the first item as requested but it doesn't actually apply it. You have to click that number and press enter to apply it.

Keavon commented 6 months ago

Oh, and more more issue with Clamped mode, sorry. If I set a single corner's radius to something big but the others are 0 or something small, the big rounded corner should extend all the way to the edge, not only 50%.

image

Expectation:

image

skoriop commented 6 months ago

One last little issue, if you switch back from Individual to Uniform, it does now display the first item as requested but it doesn't actually apply it. You have to click that number and press enter to apply it.

I've added a Message::Batched variant (as suggested by @TrueDoctor on Discord) and modified the on_update callback function of the radio input to ensure that it also updates the corner radius input.

Oh, and more more issue with Clamped mode, sorry. If I set a single corner's radius to something big but the others are 0 or something small, the big rounded corner should extend all the way to the edge, not only 50%.

For this, I modified the clamping code for the individual rounding. It first finds the largest corner radius, clamps the two corner radii adjacent to it and finally clamps the last corner radius (which is diagonally opposite to the largest corner radius).

Keavon commented 6 months ago

!build

github-actions[bot] commented 6 months ago
📦 Build Complete for 6858312300b9bf96da98bc2bcab88c01616e7689
https://42d6a2fa.graphite.pages.dev
Keavon commented 6 months ago

For this, I modified the clamping code for the individual rounding. It first finds the largest corner radius, clamps the two corner radii adjacent to it and finally clamps the last corner radius (which is diagonally opposite to the largest corner radius).

Seems like a decent algorithm you devised, although I think it's more important to remain consistent with user expectations from CSS (especially if Graphite gets used as a tool for web design mockups in the future). Could you please update it to use this algorithm?:

https://drafts.csswg.org/css-backgrounds/#corner-overlap

After that, we're all good to merge this!

skoriop commented 5 months ago

Yup, I've updated that part.

Keavon commented 5 months ago

!build

github-actions[bot] commented 5 months ago
📦 Build Complete for 484cf7c93f15c1b53025024c86cf52fbfd8324b6
https://98bd932f.graphite.pages.dev