Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support draw rounded rectangles with varying corners. #290

Merged

Conversation

GPrimola
Copy link
Contributor

@GPrimola GPrimola commented Sep 11, 2022

Description

This PR extends the already existent Scenic.Script.draw_rounded_rectangle/5 functions to 3 others:

  • Scenic.Script.draw_rounded_rectangle/6
  • Scenic.Script.draw_rounded_rectangle/7
  • Scenic.Script.draw_rounded_rectangle/8

Those extra arguments can be used to draw a rounded rectangle with varying corners radii, and it was done having in mind the HTML <canvas> spec for rounded rectangle while keeping the already existing behavior of rounded rectangle on Scenic.

This functionality was implemented using the already included function nvgRoundedRectVarying on the NanoVG's version Scenic relies on, so there's no need to update any dep's version.

This PR depends on and relates to ScenicFramework/scenic_driver_local#17.

This PR introduces no breaking changes once it doesn't change neither the interface nor the behaviour of the previous existing Scenic.Script.draw_rounded_rectangle/5.
And while it depends on ScenicFramework/scenic_driver_local#17, it won't break without it. Instead of breaking, scenic_driver_local/c_src/script.c will only emit a log warning that the operation 0x0C (draw_rrectv on Scenic) doesn't exist.

Motivation and Context

The motivation behind this PR is for Scenic.Script be more compatible with HTML specs.

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature
    but make things better)

Checklist

  • Check other PRs and make sure that the changes are not done yet.
  • The PR title is no longer than 64 characters.

Copy link
Contributor

@crertel crertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, super neat work!

lib/scenic/script.ex Outdated Show resolved Hide resolved
@GPrimola GPrimola force-pushed the extended_draw_rounded_rectangle branch from 4b446b8 to 853baba Compare August 2, 2023 07:44
Copy link
Contributor Author

@GPrimola GPrimola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@GPrimola
Copy link
Contributor Author

GPrimola commented Aug 2, 2023

Hi @crertel, can you take a look on this again? :)

Many thanks!

@GPrimola GPrimola changed the title [Proposal] Support to draw rounded rectangles with varying corners. Support draw rounded rectangles with varying corners. Aug 5, 2023
@GPrimola GPrimola requested review from crertel and boydm August 16, 2023 07:26
@GPrimola
Copy link
Contributor Author

GPrimola commented Feb 7, 2024

Hey @crertel,
on #290 (review) you added a change request.
After that I've tackled some stuff. Could you check again if the changes you requested have been tackled?

Thanks! :D

@crertel
Copy link
Contributor

crertel commented Jun 15, 2024

The other half of this is handled in ScenicFramework/scenic_driver_local#67 .

@crertel crertel merged commit e7e7e03 into ScenicFramework:main Jun 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants