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

draw rounded edges in Rectangle #1090

Closed
MJacred opened this issue Jun 9, 2020 · 47 comments
Closed

draw rounded edges in Rectangle #1090

MJacred opened this issue Jun 9, 2020 · 47 comments
Labels
blocker Items that would block a forthcoming release enhancement New feature or request help wanted Extra attention is needed

Comments

@MJacred
Copy link

MJacred commented Jun 9, 2020

Is your feature request related to a problem? Please describe:

For better readability and a more diverse collection of shapes. Different Components in Material Design have them, like Material buttons, tags/chips, cards, ...

Is it possible to construct a solution with the existing API?

desired outcome not possible at this time

Describe the solution you'd like to see:

define in canvas/Rectangle.go Rectangle struct additional members: cornerX1, cornerX2, cornerY1, cornerY2,
then define defaults in theme/theme.go per Widget: Makes sense for Button, Entry, Select, PopUp, TabContainer Button, ProgressBar, Spinner (wip). And other widgets which will probably come later: Frame, Switch.
Maybe let the user overwrite them in code?

see border radius in CSS

@andydotxyz
Copy link
Member

I’m a little torn here on whether the individual corner configuration is needed? Adding “CornerRadius” to Rectangle could be a nice addition. However if it was to be 4 new fields then it might hint towards a RoundedRectangle type that offers more flexibility?

Just some thoughts really - no conclusion other than it sounds like a good idea in principle.

@andydotxyz andydotxyz added the enhancement New feature or request label Jun 9, 2020
@MJacred
Copy link
Author

MJacred commented Jun 10, 2020

Hm.. one CornerRadius probably suffices. I proposed the 4 for maximum configuration possibility.

I'd have no problem with an extra RoundedRectangle, which embeds Rectangle. So that both work harmoniously together, we'd probably need a Rect interface which both rectangle structs implement?

@andydotxyz
Copy link
Member

The way that we add APIs here is based on a user requirement and exposing the least possible changes to support that - so maximum configuration isn't really a mantra we follow. If such configuration is needed we could add it as long as the API is kept clean and simple.

The canvas package uses only concrete types as they are used to pass through to a rendering engine so in that space interfaces don't usually work. With Go a Rectangle type could be embedded inside a RoundedRectangle - though whether that is cleaner than a small amount of duplication might be up for debate.

@fpabl0
Copy link
Member

fpabl0 commented Jan 22, 2021

@andydotxyz Isn't it possible to just add a new field to canvas.Rectangle called BorderRadius? Then the canvas.Rectangle could be any kind of rectangle rounded or not. BorderRadius could be private and defined from Theme API. I am not sure how this can be implemented but looking at internal/painter/draw.go the method DrawRectangle could use rasterx.AddRoundRect instead of rasterx.AddRect to enable border radius. This feature would be very helpful to make the current widgets more beautiful :)

@jtlapp
Copy link

jtlapp commented Oct 9, 2021

Would the dev need to implement anti-aliasing for this, or does a suitable API already exist?

I'm still making a go/no-go decision about whether to implement my app in Go + Fyne. It's possible that I could add this feature, if it isn't done by then.

@andydotxyz
Copy link
Member

To add new canvas primitives (or features to them) we need to add in the software and OpenGL renderers.
The way that antialias works varies in this case. For the software renderer you can use any drawing API that can do curved corners and that will probably anti-alias. For the OpenGL it would mean replacing the simple rectangle draw with a shader (similar to the line implementation).
You can see these things in action at internal/painter/*

@andydotxyz andydotxyz added the help wanted Extra attention is needed label Oct 22, 2021
@renlite
Copy link
Contributor

renlite commented Oct 26, 2021

In draw.go you are using module rasterx to draw Line, Rect and Circle.
Would it work, to extend draw.go with func DrawRoundedRectangle(...), which calls the func DrawRectangle(...), if no corner radius (1,2,3,4) are set as parameter?
The module rasterx has a func AddRoundRect(minX, minY, maxX, maxY, rx, ry, rot float64, gf GapFunc, p Adder) {...} which offers the functionality. (AddCircle() is already in use)

@andydotxyz
Copy link
Member

Yes that would work, but it is a fallback for when OpenGL rendering can’t deal with the object. As having no border is currently drawn as hardware accelerated texture you might need the fallback mechanism to work differently than you described.
Or add it to GL renderer as well :)

@renlite
Copy link
Contributor

renlite commented Oct 27, 2021

So far I understand the Circle object is always rendered as fallback and for OpenGL rendering of a RoundRect there would be a solution necessary like: Texture 9-slices (Triangle) or using Polygons. Is this correct?

func (p *glPainter) drawTextureWithDetails(o fyne.CanvasObject, creator func(canvasObject fyne.CanvasObject) Texture,
	pos fyne.Position, size, frame fyne.Size, fill canvas.ImageFill, alpha float32, pad float32) {

	texture := p.getTexture(o, creator)
	if texture == NoTexture {
		return
	}

	aspect := float32(0)
	if img, ok := o.(*canvas.Image); ok {
		aspect = painter.GetAspect(img)
		if aspect == 0 {
			aspect = 1 // fallback, should not occur - normally an image load error
		}
	}
	points := p.rectCoords(size, pos, frame, fill, aspect, pad)
	vbo := p.glCreateBuffer(points)

	p.glDrawTexture(texture, alpha)
	p.glFreeBuffer(vbo)
}

func (p *glPainter) drawCircle(circle *canvas.Circle, pos fyne.Position, frame fyne.Size) {
	p.drawTextureWithDetails(circle, p.newGlCircleTexture, pos, circle.Size(), frame, canvas.ImageFillStretch,
		1.0, painter.VectorPad(circle))
}

...

func (p *glPainter) drawRectangle(rect *canvas.Rectangle, pos fyne.Position, frame fyne.Size) {
	if (rect.FillColor == color.Transparent || rect.FillColor == nil) && (rect.StrokeColor == color.Transparent || rect.StrokeColor == nil || rect.StrokeWidth == 0) {
		return
	}
	p.drawTextureWithDetails(rect, p.newGlRectTexture, pos, rect.Size(), frame, canvas.ImageFillStretch,
		1.0, painter.VectorPad(rect))
}
```

@andydotxyz
Copy link
Member

You are right that Circle still needs to have acceleration added.
You can see the Line object which does have OpenGL code - which is more desirable of course.
Rectangle is so heavily used I would worry about moving it to a slower renderer

@renlite
Copy link
Contributor

renlite commented Nov 30, 2021

During my attempts to draw a rectangle as primitive GLObject I realized that drawLine(...), drawRectangle(...) and so on are called twice on first and following render process with same param values. Is this necessary for layout or rendering?

I switched the original drawRectangle(...) to fragmentshader and vertexshader (GLSL) and the try seems to work.
image
Next step would be to try to implement a drawRoundRectangle(...) method with 9-slices.

I saw there is the lib raylib, which offers all the core gl functionality in a easier way and many base shapes (RoundRec), that are rendered as vertexshader. Raylib supports many platforms and OpenGL/ES versions. Wouldn't it be easier to use core modules of raylib for all backend OpenGL rendering?

@andydotxyz
Copy link
Member

During my attempts to draw a rectangle as primitive GLObject I realized that drawLine(...), drawRectangle(...) and so on are called twice on first and following render process with same param values. Is this necessary for layout or rendering?

They should not be called twice with identical values as we have a cache to handle this.
It is possible that the object values were the same but the canvas scale or texture scale were changed due to the window being realised on one screen then moved to another by the window manager (for example).

Wouldn't it be easier to use core modules of raylib for all backend OpenGL rendering?

I don't know the library but it looks like C code and our OpenGL work is using the Go bindings - I would prefer to keep it out of C as much as possible, unless there is a sensible reason like a large performance gain.

@andydotxyz
Copy link
Member

I switched the original drawRectangle(...) to fragmentshader and vertexshader (GLSL) and the try seems to work.

Do remember to double check that you have not invoked with a borderless rectangle, that already uses OpenGL shaders (it is only when a border radius is set with non-transparent border that it would use the old software fallback).

@renlite
Copy link
Contributor

renlite commented Nov 30, 2021

I have changed only in https://github.com/fyne-io/fyne/blob/master/internal/painter/gl/draw.go the drawRectangle(...) to:

func (p *glPainter) drawRectangle(rect *canvas.Rectangle, pos fyne.Position, frame fyne.Size) {
	println("***Rectangle***")
	points := p.rectCoords(pos, rect.Size(), rect.StrokeWidth, frame)
	vbo := p.glCreateRectBuffer(points)
	p.glDrawRect()
	p.glFreeBuffer(vbo)
}

And I use canvas.NewRectangle() or widget.NewButton(). You are right only the Button ist called twice, sorry.
image

raylib has many language bindings and for golang too. I don't know what the overhead would be, but maybe a fallback would not be necessary then.

@Jacalz
Copy link
Member

Jacalz commented Nov 30, 2021

raylib has many language bindings and for golang too. I don't know what the overhead would be, but maybe a fallback would not be necessary then.

That is not going to be a working solution in our case. Linking to C-projects unfortunately introduces another dependency that needs to be installed at both compile and runtime. It it does not quite work as regular Go code and because of our effort to be as statically linked as possible, we need to be very careful with new dependencies (or don’t add them at all).

@andydotxyz
Copy link
Member

That is not going to be a working solution in our case. Linking to C-projects unfortunately introduces another dependency that needs to be installed at both compile and runtime.

Unless it is statically compiled in like with GLFW :)

@Bluebugs
Copy link
Contributor

Bluebugs commented Dec 1, 2021

I would like to point out that there are cost and risk to consider by pulling non go dependency in. The big one, is that you now depend on another community to provide what you need. You also need to know that other language. Past experience, tell me that crossing language barrier means few people will be able to contribute and help address issues. Would really prefer a go dependencies here for that reason.

@Jacalz
Copy link
Member

Jacalz commented Dec 2, 2021

That is not going to be a working solution in our case. Linking to C-projects unfortunately introduces another dependency that needs to be installed at both compile and runtime.

Unless it is statically compiled in like with GLFW :)

That might make the binaries significantly bigger if the projects are very large. I don’t know if it even can strip out the unused parts of the C code. Probably not.

@renlite
Copy link
Contributor

renlite commented Jan 8, 2022

Last days i implemented an alpha version of a round renctangle as primitive GLObject in the fork: https://github.com/renlite/fyne
It is a prototype and base work you could build on.
Following files are changed in the fork-repository:

  • geometry.go
  • canvas/rectangle.go
  • internal/painter/gl/gl_core.go
  • internal/painter/gl/painter.go
  • internal/painter/gl/draw.go
  • example/roundrect.go (NEW)

Some notes:

  • It is just a prototype for testing (perfromance?, shapes behave correctly?); e.g. no code optimizations, no proper error handlich (my first go code)
  • All corners are calculated, maybe a mirroring of normalized vertices of one corner could be more efficient.
  • StrokeWith is not implemented. Idea: Calculation of a smaler GLObject in another color.
  • In RectangleRadius are all 4 corners defined, but only the left corners and the right corners could be different. It seems that an implementation of different 4 corners would be more complicated and would need more slices. An RoundHRectangle and RoundVRectangle could offer more flexibility. This implementation of RoundRect is a horizotal version.
  • Not only the redius but also the left and right side segments of the corners can be different. If you omit the difinition of LeftSegments or RightSegments parameters 8 segments are the default value.
  • Radius is defined in percentage. I am not sure this is a good idea.
  • I played with the radius and the segments, the file example/roundrect.go shows some possibilities. These are not fyne canvas compositions. See the code and screenshot of the example. https://github.com/renlite/fyne/blob/master/example/roundrect.go

image

  • It should be possible to paint a circle with RoundRectangle but because of performance IMO there should be a separate implementation.

@andydotxyz
Copy link
Member

This is super cool, thanks for working on this.
I think we should hide the concept of segments though - the examples when you use it is more like a path than a rounded rectangle, and if not set appropriately it looks blocky. If required in the implementation I think we should automatically work it out internally.

Agreed that percentage isn't intuitive. If you have a rectangle height 10 with radius 5 it should join in the middle.
Also I think it would be cleaner if the default constructor assumed all radii were the same, simply passing a radius float32 which will be used appropriately within the constructor.
Last thought - can we antialias the edge?

@renlite
Copy link
Contributor

renlite commented Jan 10, 2022

I turned antialias on but I don't see much difference? Does it work?
glfw.WindowHint(glfw.Samples, 4)
gl.Enable(gl.MULTISAMPLE)

image

@Bluebugs
Copy link
Contributor

Bluebugs commented Jan 10, 2022

Just a few comments here. I agree with @andydotxyz, having segment in the rounded rectangle primitive seems to be a work around from not having a path primitive on the canvas and make the API harder than it should be.
Also please do not turn on MULTISAMPLE. It is making everything a lot more costly to run and will impact hardware that run on battery tremendously (reducing performance on other platform too). I do not think it is necessary to have it on to manage to get a smooth edge, but it would require to do the anti-aliasing manually (via the shaders).

@Bluebugs
Copy link
Contributor

@renlite I quickly read through your change on your tree and it seems you have addressed our past concern. Would you mind explaining a little bit how you are doing things as the commit are a little bit difficult to follow at the moment. I would expect you would rebase/squash/make independent meaningful commits before doing a PR, but before doing that additional work just doing a quick description of what you have done will be easier and quicker.

@renlite
Copy link
Contributor

renlite commented Feb 13, 2022

  • "lineCoords" is reused for antialiasing of the round parts in the method "flexLineCoords". For every segment a line is painted, to get the effect. The GLSL code is updated to calculate the aplpha value only for the lines. (An explanatory picture will follow.)
  • In the first version I used "lineCoords" with an optinal parameter, not to break actual implementation. As you see the code is almost identical. The main difference is that the scaling and positioning is done in "flexRectCoords" already. I dont like code repetition but at the moment I don't understand the calculation logic in "lineCoords" completly, so maybe later I could wirte my own calculation for the half line width, because only the outer side is relevant for antialiasing of the circle segments.
  • The antialiasing works only for colors with alpha 255, no transparent colors. In the example "flexrect.go" you can try it with the identical colors blue_gray(A: 180) or blue_gray1(A: 255). Short, the reason is overlapping of line and circle segment on some pixels.
  • I tried some calculations with radius, linewidth and feather. Now the results seem to be OK. Would you please test it on different resolutions and give me a feedback.
  • I renamed the shape to flexRect, because it is not only "round" with the ability to use segments.
  • For a round rectangle with the same radius on all edges I have an idea to implement first a version of an antialiased circle without "lineCoords" and if it works to reuse it for a stand alone roundRect shape. As I never heard form GLSL before I don't know if I would be able to realize it.

If the results are OK for you the next steps would be to comment the code and perhaps to paint a smaler inner shape to be able to define the stroke of the shape, which doesn't work at the moment.

@andydotxyz
Copy link
Member

This is really amazing work thanks.
Just one thought on your comments above - you say next steps include "next steps would be to comment the code " but you should know (more info in our Contributing docs) that we prefer readable code over comments - mantra "If you need a comment the code is not clear enough" (there are just a couple of notable exceptions and graphics may sometimes be just that).

@renlite
Copy link
Contributor

renlite commented Feb 14, 2022

@andydotxyz
Yes, you're right. What I mean is a short description in front of the method.

@Bluebugs
image
image
image
image
image

Alpha not 255.0:
image
blue_gray := color.NRGBA{R: 83, G: 140, B: 162, A: 180}

image
blue_gray1 := color.NRGBA{R: 134, G: 174, B: 189, A: 255}

@Bluebugs
Copy link
Contributor

Thanks @renlite, that looks pretty good actually. Regarding your point about only supporting opaque colours, I would believe it would make the shader slightly more complex to address it, but should be doable. Is it something you can look at or will you need help looking at it?

@Bluebugs
Copy link
Contributor

I think I didn't really share my idea well for the shader, but maybe adding another parameter that describe which of the 4 quarter of a circle need to be drawn would work for this.

@renlite
Copy link
Contributor

renlite commented Feb 22, 2022

@Bluebugs Sorry for late reply I don't have much time at the moment to share my two ideas / directions to solve the transparent color problem.
Could you please explain what you mean with extra parameter for the "4 quarter of a circle" for the fragment shader.

I think the problem is the overlapping of some line pixels with some circle segment pixels. That happens because I am using the original line implementation which paints the inner and outer stroke + feather of the line. Maybe a new line rendering painting only the outer side of a line could help to draw the line without overlapping the circle parts. I'm not sure if its possible to paint the line exactly along the edge of the circle without empty pixels at some positions.

image

In other case the fragment shader should know if a pixel is overlapping and in this case no color should be painted? Maybe there is a solution with GLSL?

@renlite
Copy link
Contributor

renlite commented Feb 23, 2022

@Bluebugs
Help is welcome for FragmentShader of FlexRectangle. A solution for "Alpha < 255" in GLSL without the need to chnage the
current go-code - as described above - would be cool.

    #version 110
    uniform vec4 color;
    uniform float lineWidth;
    uniform float feather;

    varying vec2 delta;

    void main() {
	   if ( delta != vec2(0.0, 0.0) ){
        	float distance = length(delta);

		if (distance <= lineWidth - feather) {
				gl_FragColor = color;
			} else {
				gl_FragColor = vec4(color.r, color.g, color.b, mix(color.a, 0.0, (distance - (lineWidth - feather)) / feather));
			}
	    } else {
			gl_FragColor = color;
		}
    }

@Bluebugs
Copy link
Contributor

It might not actually be possible. I was thinking of using discard in the shader to just not do anything which would mean there would not have been any conflict for that pixels, but I don't see how you could be precise enough to do so.

I think the solution is only if the outline can be drawn in the same shader that does the rectangle. This way only one pixel is every pushed to the screen and the blending would work. Not to sure how that would fit in your work.

@Bluebugs
Copy link
Contributor

I actually did get an idea last night that would work and shouldn't be to hard to implement. It does require shader change still. My idea would be to render the outline in a 8bits buffer and use it as a mask for the rounded rectangle shader. The rounded rectangle shader would receive both the color of the outline and the fill color too. This way when deciding to color a point, it can check if the mask color is set, in which case it would be a function of mask * outline color + (1 - mask) * gl_FragColor.

@renlite
Copy link
Contributor

renlite commented Feb 28, 2022

Sorry, but I don't understand your idea "to render the outline in a 8bits buffer and use it as a mask for the rounded rectangle shader". Could you please explain it further or give me an example what you mean.

Yesterday I started to implement a new version of the line rendering with stroke and feather only on one side. That should make it possible to draw the base line (of the line) exactly on the outline of the circle segement and then to use stroke + feather for antialiasing. In that case I would change the FragmentShader to transparent color (Alpha 0,0) and if necessary adapt the if statement for (distance <= ...). Do you think it could work?

if (distance <= lineWidth - feather) {
>>> 	gl_FragColor =  vec4(0.0, 0.0, 0.0, 0,0)    <<<
	} else {
		gl_FragColor = vec4(color.r, color.g, color.b, mix(color.a, 0.0, (distance - (lineWidth - feather)) / feather));
	}

But I would like to understand your solution.

@Bluebugs
Copy link
Contributor

I was thinking of creating a FBO and use a texture with only one component, like GL_RED_INTEGER, as a render target. Using that FBO to render the outline of the button in that texture and then the resulting texture can be used in the rounded rectangle shader to render the outline and the inside content without "double" rendering and avoid the alpha problem when rendering the pixel twice.

For reference a good introduction on FBO: https://learnopengl.com/Advanced-OpenGL/Framebuffers

Your idea might work, I am not certain about rounding between two execution, but likely it should work too. At least at high level, I think it is likely to work.

@renlite
Copy link
Contributor

renlite commented Mar 2, 2022

After some thinking I only changed the original method flexLineCoords(), to paint only the outside of the line (FramentShader is not changed):

	return []float32{
		// coord x, y normal x, y
		x1, y1, normalX, normalY,
		x2, y2, normalX, normalY,
		x2, y2, 0.0, 0.0,         <<<
		x2, y2, 0.0, 0.0,         <<<
		x1, y1, normalX, normalY,
		x1, y1, 0.0, 0.0,         <<<
	}, halfWidth, featherWidth

As far as I can see the transparent colors and anitaliasing seem to work too.

image

https://github.com/renlite/fyne
Would you please check it with other resolutions and other variations?

image
image
image
image

@andydotxyz
Copy link
Member

Nice! Great work :)

@renlite
Copy link
Contributor

renlite commented Apr 22, 2022

https://github.com/renlite/fyne
@andydotxyz @Bluebugs
Before I make a pull request I would like to share some information about the new version:
image

  • After the last version I thought it would be sufficient to create shapes with stroke through canvas composition. This is possible but it doesn't work correctly with Alpha < 255. As you can see on the picture below the round rectangle in the example mixes the color from the backround (=strokeColor orange) with the color of foreground because the yellow front color is transparent. So if the front color is opaque you can use composition of two objects for stroke rendering. The left rectangle is only one object including primitive GLObjects.
  • I had to rethink the implementation and expand the rendering with and without stroke. I expect this new rendering should be faster because of only one communication with OpneGL.
  • After some experiments I cleaned up the code and have rewritten the func NewRectrangle(...) with 'Options pattern'.
  • func (p *glPainter) drawRectangle(...) was commented out and replaced with own version. In the example I tried the "Button" widget with a fix color in the func (b *Button) CreateRenderer(), but it doesn't work. No color and no click callback. Could you please have a look on it what's wrong.
  • In the func (p *glPainter) glDrawRect(...) I have implemented println("glDrawRect) for debugging. The function is invoked twice for every canvas object. Please check it if you have the same results and why it is called twice.
  • Would you please check the antialiasing with other resolutions.
  • Shapes with stroke and only one segment don't work properly. For shapes with opaque colors you could use composition. I think I know the reason but as this is not the main use for this canvas object I don't plan to implement it.

Would you please inform me if the branch is ok for a pull request.

@andydotxyz
Copy link
Member

This looks really great thanks - very promising indeed.
I'm not sure about the options pattern though, we don't use it any where else, preferring constructor functions and field manipulation - smaller APIs that are easier to discover IMHO. (plus we can't make breaking changes to the existing NewRectangle function.

To feed back on the CreateRenderer issue can you please point to the line of code that's not working?

@andydotxyz
Copy link
Member

I tried to compile but get this:

❯ go run .
panic: failed to compile 
    #version 110
    attribute vec2 vert;
    attribute vec2 normal;
	attribute float switch;
	attribute float lineWidth;
	attribute float feather;
    
    varying vec2 delta;
	varying float switch_var;
	varying float lineWidth_var;
	varying float feather_var;

    void main() {
		switch_var = switch;
		lineWidth_var = lineWidth;
		feather_var = feather;
		if ( normal == vec2(0.0, 0.0) ) {
			gl_Position = vec4(vert, 0, 1);
		} else {
			delta = normal * lineWidth_var;
			gl_Position = vec4(vert + delta, 0, 1);
		}
    }
: ERROR: 0:5: 'switch' : Reserved word. 
ERROR: 0:5: 'switch' : syntax error: syntax error


goroutine 21 [running, locked to thread]:
fyne.io/fyne/v2/internal/painter/gl.(*glPainter).Init(0xc000702040)
	/tmp/fyne/internal/painter/gl/gl_core.go:303 +0x35b
fyne.io/fyne/v2/internal/driver/glfw.(*window).create.func2()
	/tmp/fyne/internal/driver/glfw/window.go:1469 +0x6b
fyne.io/fyne/v2/internal/driver/glfw.(*window).RunWithContext(0xc000126600, 0xc000012010)
	/tmp/fyne/internal/driver/glfw/window.go:1354 +0x4f
fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).startDrawThread.func1()
	/tmp/fyne/internal/driver/glfw/loop.go:232 +0x14c
created by fyne.io/fyne/v2/internal/driver/glfw.(*gLDriver).startDrawThread
	/tmp/fyne/internal/driver/glfw/loop.go:224 +0xef
exit status 2

@renlite
Copy link
Contributor

renlite commented Apr 26, 2022

Thanks for your feedback.
Sorry, I don't know why the GLSL compiler doesn't work on your machine. I changed the attribute switch to colorSwitch. Now it should work.

	vertexRectShaderSource = `
    #version 110
    attribute vec2 vert;
    attribute vec2 normal;
    attribute float colorSwitch;
    attribute float lineWidth;
    attribute float feather;

The reason I have selected the options pattern is not to break the original NewRectangle API and to offer further options in simple form. The new version could be used as before and can be extended with new and defined options. If I change the param to type Rectangele struct, I will break the existing code. My first implementation was:

func NewRectangle(color color.Color) *Rectangle {}
// #1
func NewRectangleWithRadius(color color.Color, radius fyne.RectangleRadius) *Rectangle {}
func NewRectangleWithStroke(color color.Color, ...) *Rectangle {}
func NewRectangleWithRadiusAndStroke(color color.Color, ...) *Rectangle {}
// #2
func NewFlexRectangle(rect Rectangle) *Rectangle {}
// #3 options patter (WithStroke and WithRadius are optional)
NewRectangle(yellow, WithStroke(orange, 5.0), WithRadius(rectRad9))

Should you prefer an other implementation, please give me the names and a short API description.

Button not working:

func (b *Button) CreateRenderer() fyne.WidgetRenderer {
	b.ExtendBaseWidget(b)
	seg := &TextSegment{Text: b.Text, Style: RichTextStyleStrong}
	seg.Style.Alignment = fyne.TextAlignCenter
	text := NewRichText(seg)
	text.inset = fyne.NewSize(theme.Padding()*2, theme.Padding()*2)

	>>>    background := canvas.NewRectangle(color.NRGBA{R: 255, G: 200, B: 0, A: 180})      <<<
        ...
        r := &buttonRenderer{
		        ShadowingRenderer: widget.NewShadowingRenderer(objects, shadowLevel),
		        background:        background,
		        tapBG:             tapBG,
		        button:            b,
		        label:             text,
		        layout:            layout.NewHBoxLayout(),
	        }

To see if a button works I changed the canvas.NewRectangle(color.NRGBA{R: 255, G: 200, B: 0, A: 180}) to a fix color.

@andydotxyz
Copy link
Member

The new version could be used as before and can be extended with new and defined options. If I change the param to type Rectangele struct, I will break the existing code.

I don't quite understand this sorry. Why was using Rectangle fields like we have for the other features not working?

I changed the attribute switch to colorSwitch. Now it should work.

This is now working thanks, though when I resize the window I get some artefacts (macOS) - have you seen this too?
Screenshot 2022-05-02 at 14 27 16

To see if a button works I changed the canvas.NewRectangle(color.NRGBA{R: 255, G: 200, B: 0, A: 180}) to a fix color.

It may be because the Button code sets background colour to match theme, which is transparent by default.

@renlite
Copy link
Contributor

renlite commented May 2, 2022

As you can see on Win10 it works without some artefacts after resize of the window. It is difficult to understand because the below rectangles (yellow/orange)

	// >> composition
		rr6,
		rr7,
		txt1,
		// >>
		rr9,

are painted correctly and it happens mostly on the right bottom. This is slice 9 which ist the last coords calculation. Could you please test it only with two objects or check it on linux or mobile?

image

I noticed that the method func (p *glPainter) glDrawRect(fillCol color.Color, strokeCol color.Color, trianglePoints int32) is called twice for every object. The call stack starts in painter.go method func (p *glPainter) Paint(obj fyne.CanvasObject, pos fyne.Position, frame fyne.Size).

It seems that in a container.NewWithoutLayout the button does not work. With BoxLayout the border of the buttons and the animation is visile.
image

My idea was if I change the original method from
func NewRectangle(color color.Color) *Rectangle
to
func NewRectangle(r *Rectangle) *Rectangle
it will break for example the existing code in

func (b *Button) CreateRenderer() fyne.WidgetRenderer{
...
	background := canvas.NewRectangle(theme.ButtonColor())
	tapBG := canvas.NewRectangle(color.Transparent)
...
}

A second or third parameter would break the existing code too, because canvas.NewRectangle is called only with one (color) parameter. Not to break the existing code I thought to implement one init method like
func NewRectangle2(r *Rectangle) *Rectangle
or more specific methods like
func NewRectangleWithStroke(fillColor color.Color, strokeColor color.Color, strokeWidth float32) *Rectangle and
func NewRectangleWithRadius(fillColor color.Color, radius fyne.RectangleRadius) *Rectangle and
func NewRectangleWithRadiusAndStroke(,,,) *Rectangle for all variations.

func NewRectangle(color color.Color, options ...RectangleOption) *Rectangle does not break existing code and offers static typed optional parameters with code completion. Only defined func can be used and further options could be added later.

Maybe func NewRectangle2(r *Rectangle) *Rectangle is the better option?

@andydotxyz
Copy link
Member

As you can see on Win10 it works without some artefacts after resize of the window.

Perhaps it can be debugged during a PR?

It seems that in a container.NewWithoutLayout the button does not work.

Did you remember to add the appropriate Move and Resize calls? without them an item is not layed out and so tap targets likely 0x0.

Maybe func NewRectangle2(r *Rectangle) *Rectangle is the better option?

I don't understand why you would have the same input and output type for a constructor function...?

func NewRectangle(color color.Color, options ...RectangleOption) *Rectangle does not break existing code and offers static typed optional parameters with code completion.

Technically yes, but the last parameter would only be "Since: 2.3" but these annotations are per-function (or type).
Overall I don't see why we need a new pattern for constructing, the current situation with structs and fields is used everywhere else and is also static typed with code completion.

@andydotxyz
Copy link
Member

This is now fixed on developed - @renlite is a real star :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Items that would block a forthcoming release enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

8 participants