-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
rectangle shadow #5841
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
base: develop
Are you sure you want to change the base?
rectangle shadow #5841
Conversation
|
By default, adding a shadow currently impacts the geometry: the canvas size is reduced by the shadow paddings so that both the object and its shadow fit inside the requested frame. This means the content (e.g., a rectangle) is shrunk to ensure the shadow does not overflow the frame. To address this, I introduced the Example CalculationsExample 1Suppose you have a rectangle with a requested frame size of 150 x 250.
Shadow paddings calculation:
After clamping negatives to zero:
Example 2Suppose you have a rectangle with a requested frame size of 100 x 100.
Shadow paddings calculation:
After clamping negatives to zero:
Notes:
This approach allows developers to choose whether the shadow should be included inside the original frame (shrinking the content) or expand the overall bounds to preserve the content size. Please let me know if you’d like further clarification or adjustments to this behavior! |
|
Copying from the wider discussion as it pertains to the API design here.
I don't think the presence of a shadow should change how the geometry of a widget or shape is interpreted at all. With the maths illustrations above complexity goes up substantially and widgets with shadows will no longer line up with others which breaks the simplicity of our layout model. |
Just FYI on that one, the bug is not about the drop of the shadow or its depth - the report is that a menu popping up from a menu bar should be at the same level so there should be shadow on 3 sides of the menu and the underside of the bar, but not between them (top of the menu) |
canvas/base_shadow.go
Outdated
| // with the input size. Total size is larger than the input size. | ||
| SizeAndPositionWithShadow(size fyne.Size) (fyne.Size, fyne.Position) | ||
| // ShadowPaddings returns the paddings (left, top, right, bottom) for the shadow. | ||
| ShadowPaddings() [4]float32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like the only one of the interface methods that adds (rather than works around geometry complications).
However I wonder if ShadowOffset and ShadowDepth really are all we need to draw shadows around objects - all these calculations could be removed and made a rendering only complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So all calculations should be moved to painter draw.go file ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interface has been updated. Now only the ShadowPaddings method is available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. As noted on the recent review now that it is reduced to padding calculations it seems not needed at all...
canvas/base_shadow.go
Outdated
| type baseShadow struct { | ||
| baseObject | ||
|
|
||
| ShadowColor color.Color // Color of the shadow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One API consideration for this PR should be:
- Is the shadow a themed colour or a canvas feature
Which impacts how it should be implemented. If widgets are to have shadows it must be in the theme - but for canvas objects you get lower level control. Worth considering in the bigger picture how this replaces the existing (gradient) shadow implementation.
We should replace old with new in the scope of this PR rather than having two different shadow implementations - plus it will inform the correct API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about gradient shadow implementation if only one-side shadow is needed like for scroll ? I think it still may be useful, but I could be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes a fair point - but for the major shadow usages (where it is a complete shadow) we should replace and test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced ShadowingRenderer and updated tests. I started with some initial values for shadow parameters which should be adjusted to make shadow looking good. I still need to update macos and mobile tests. I can start fixing tests once final shadow parameters per object (dialogs, widgets, etc.) are determined.
|
Very cool indeed thanks. I have made a few API /design comments inline. Another thing to note would be that we have also the software renderer which would need to understand this new feature and draw something similar for compatibility. |
You're right, the top shadow is the problem there. I just wanted to point out that a rectangular shadow may help with that.
I also prefer simple solutions :) To be honest, I am not an expert in such graphical implementations. The biggest challenge was to draw the shadow correctly and that's what I focused all my attention on. However, all changes in geometry were made only to "see" the desired effect, because without any change the "frame" clipped the shadow and I was not able to assess whether it was drawing correctly. That's why I am open to any suggestions regarding the API and changes on the library side. |
This will be a limitation of the rendering and not part of the Fyne geometry design. If you look at rectangle, line and others you will see a |
That would be great, so I can just extend I don't fully understand how Fyne's design geometry works, and that's why I added so much unnecessary code. |
Yes
In essence it is really simple - a device agnostic coordinate system so it's consistent across all devices. Every item is positioned relative to the parent. More info at https://docs.fyne.io/architecture/geometry |
|
I added shadow to software painter along with test cases |
Thanks, I appreciate that when packed together it looks peculiar but I think this is better for the geometry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working more on this, it is coming together.
I have a few questions at this time:
- Isn't the geometry in the software painter wrong? The objects are moving out of their position to accommodate the shadow
- The Shadowable interface isn't clear - what is its purpose? It doesn't seem to be needed
- The public
Shadowtype can only be used as embedded in other types - what are your thoughts on this balance?
| if shadowColor != color.Transparent && shadowColor != nil && (!shadowOffset.IsZero() || shadowSoftness > 0.0) { | ||
| bounds = drawShadow(c, obj, fyne.NewSize(width, height), shadowSoftness, shadowOffset, shadowColor, 0, bounds, base, clip) | ||
| // due to shadow draw rectangle with a certain width and height | ||
| raw := painter.DrawRectangle(canvas.NewRectangle(fill), width, height, 0, func(in float32) float32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this - is it the cause of test failures?
If the geometry is not changed then isn't it the same rectangle but drawn at a different offset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to accomodate shadow the frame must be extended, without this change rectangle always fill in the entire space and the shadow is not visible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there is issue on macos platform, could you please show me the rendered images on macos so I can try to investigate the problem because on my local windows platform all tests succeed.
canvas/shadow.go
Outdated
| // Since: 2.7 | ||
| type Shadowable interface { | ||
| // ShadowPaddings returns the paddings (left, top, right, bottom) of the shadow. | ||
| ShadowPaddings() [4]float32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be derived from the public shadow struct data - what is the purpose of having a new interface with this method when it could be internal to the rendering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interface is useful when I have obj fyne.CanvasObject as an argument and want to quickly parse it to the new interface s, ok := obj.(canvas.Shadowable) to get paddings ShadowPaddings() but maybe there is a better approach without using new interface.
canvas/rectangle.go
Outdated
| // Rectangle describes a colored rectangle primitive in a Fyne canvas | ||
| type Rectangle struct { | ||
| baseObject | ||
| // Support shadow configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Embedding at the beginning of the struct like this will break any code that has done &Rectangle{color.Transparent} - though that is not best practice we try to avoid breaking if we can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, I will move the embedding at the end of the struct
Thanks for review.
Which implementation is expected ? b (public field). |
|
I have removed Containers:
Widgets:
Internal GLFW:
I would like to highlight that I set some initial values for the shadow softness and offset to show what it might look like and how the test might change. I was able to update some platform tests to be successful. |
|
If you don't mind, I'd like to continue working on this feature for version 2.8. |

Description:
(Based on #5830)
Fixes:
Shadow Support for Rectangle, Square, and Circle
Introduced a
Shadowstruct andShadowableinterface to provide reusable shadow logic.Added fields for shadow color, softness, offset, and type (drop/box shadow).
TheExpandForShadowfield allows the object to keep its requested/original size, expanding the total bounds to include the shadow.Updated shader and painter logic to render shadows with configurable softness and offset, and to blend shadows correctly.
If a canvas object has a shadow added, the canvas size is reduced by the shadow paddings on each side. This ensures that the entire object—including its shadow—fits within the originally requested frame size. The content area (without shadow) will be smaller to accommodate the shadow within the allocated space.If a canvas object has a shadow added, the canvas coords are expanded to accommodate the shadow.Shader and Painter Updates
API Additions
Status
Additional Notes
This work is a prerequisite for adding shadows and rounded corners to popups, dialogs, and menus in Fyne. I have already applied these changes to those components and have working code locally. Rectangle shadow provides the necessary foundation for theme-level enhancements.
Examples
Dialog, menu shadow (preview)
Please let me know if you have any suggestions, concerns, or requests for changes.
Checklist:
Where applicable: