fyne icon indicating copy to clipboard operation
fyne copied to clipboard

draw rounded edges in Rectangle

Open MJacred opened this issue 4 years ago • 46 comments

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

MJacred avatar Jun 09 '20 19:06 MJacred

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 avatar Jun 09 '20 22:06 andydotxyz

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?

MJacred avatar Jun 10 '20 10:06 MJacred

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.

andydotxyz avatar Jun 10 '20 13:06 andydotxyz

@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 :)

fpabl0 avatar Jan 22 '21 00:01 fpabl0

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.

jtlapp avatar Oct 09 '21 13:10 jtlapp

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 avatar Oct 10 '21 16:10 andydotxyz

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)

renlite avatar Oct 26 '21 18:10 renlite

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 :)

andydotxyz avatar Oct 26 '21 19:10 andydotxyz

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))
}
```

renlite avatar Oct 27 '21 10:10 renlite

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

andydotxyz avatar Oct 27 '21 11:10 andydotxyz

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?

renlite avatar Nov 30 '21 12:11 renlite

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 avatar Nov 30 '21 13:11 andydotxyz

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).

andydotxyz avatar Nov 30 '21 13:11 andydotxyz

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.

renlite avatar Nov 30 '21 14:11 renlite

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).

Jacalz avatar Nov 30 '21 15:11 Jacalz

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 :)

andydotxyz avatar Dec 01 '21 21:12 andydotxyz

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.

Bluebugs avatar Dec 01 '21 22:12 Bluebugs

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.

Jacalz avatar Dec 02 '21 08:12 Jacalz

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.

renlite avatar Jan 08 '22 22:01 renlite

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?

andydotxyz avatar Jan 10 '22 03:01 andydotxyz

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

image

renlite avatar Jan 10 '22 11:01 renlite

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 avatar Jan 10 '22 16:01 Bluebugs

I turned antialias on but I don't see much difference? Does it work?

Yes, that looks much less jagged.

andydotxyz avatar Jan 10 '22 19:01 andydotxyz

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).

I guess there may be some inspiration available in the line renderer that was contributed which does anti-aliasing without the multisample enabled.

andydotxyz avatar Jan 10 '22 19:01 andydotxyz

New version with antialiasing and some changes. https://github.com/renlite/fyne image

renlite avatar Feb 12 '22 16:02 renlite

@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.

Bluebugs avatar Feb 12 '22 19:02 Bluebugs

  • "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.

renlite avatar Feb 13 '22 20:02 renlite

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).

andydotxyz avatar Feb 13 '22 21:02 andydotxyz

@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}

renlite avatar Feb 14 '22 08:02 renlite

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 avatar Feb 14 '22 15:02 Bluebugs