Add OpenGL (ES)/GPU rendered rectangle and rectangle with rounded edges #3539

renlite · 2023-01-04T23:56:44Z

Description:

This PR replaces the PR #3121. The difference to this PR is the technique how the primitive OpenGL objects are rendered. In the PR #3121 the objects were mainly calculated on the CPU whereas in this PR the objects are primarily calculated on the GPU.
The new OpenGL rendering should bring better performance and the possibility to paint antialized rectangles with rounded edges. With this technology it's possible to paint a circle. In a second step the texture based circle could be switched too.
Thanks to: @Bluebugs for support and @andydotxyz @ErrorNoInternet @Jacalz for tests.

Fixes issue #1090

Checklist:

Tests included.
Lint and formatter run with no errors.
Tests all pass.

Bluebugs

Looking pretty good. Just a small more cleanup in my opinion. I will try to dedicate some time for testing on mobile and web later this week.

Bluebugs · 2023-01-05T00:23:45Z

cmd/rect_test/main.go

@@ -0,0 +1,79 @@
+package main


The "Tests" checkbox is actually for a test that would pass via go test, not a binary to manually check the result. Actually what is in this file should likely be included in fyne_demo/canvas section.

Indeed. This file would have to be removed (and then have fyne_demo updated instead) as we shouldn't add more applications into the cmd/ directory.

Yes, this is still an open point. I haven't had time yet.
Next steps will be:

Test implementation

Extend fyne_demo

Delete cmd/rect_test

In the meantime you can use the rect_test for web and mobile testing.

Open points done.

Bluebugs · 2023-01-05T00:24:42Z

internal/painter/gl/draw.go

@@ -83,7 +82,11 @@ func (p *painter) drawObject(o fyne.CanvasObject, pos fyne.Position, frame fyne.
 	case *canvas.Raster:
 		p.drawRaster(obj, pos, frame)
 	case *canvas.Rectangle:
-		p.drawRectangle(obj, pos, frame)
+		if o.(*canvas.Rectangle).Radius.Pixel == 0 && o.(*canvas.Rectangle).Radius.Percent == 0 {


internal/painter/gl/draw.go

Bluebugs · 2023-01-05T00:27:11Z

internal/painter/gl/draw.go

@@ -231,6 +369,66 @@ func (p *painter) lineCoords(pos, pos1, pos2 fyne.Position, lineWidth, feather f
 	}, halfWidth, featherWidth
 }

+/*
+func (p *painter) flexLineCoordsNew(pos, pos1, pos2 fyne.Position, lineWidth, feather float32, frame fyne.Size) ([]float32, float32, float32) {


We prefer to not have dead code. Would you mind removing this until it is of use?

Not used code deleted - done.

coveralls · 2023-01-05T00:41:09Z

Coverage: 61.798% (-0.3%) from 62.085% when pulling 0bdbe90 on renlite:develop into 78b8193 on fyne-io:develop.

andydotxyz

I like how this is shaping up - and agree with the other comments.
However I don't agree with the need for a Radius type - and I certainly don't think we should use pixel definition in the middle of a vector toolkit.

andydotxyz · 2023-01-06T21:36:07Z

geometry.go

+
+// RectangleRadius describes a radius in pixel or in percentage
+type Radius struct {
+	Pixel   float32


I don't think this is a good idea - the toolkit API is all vector and therefore pixel parameters should not be used.

Can we just use float32 instead of Radius - the Width and Height are the same so that will be what developers are expecting. We also don't have percentage based calculations elsewhere so I don't think it's needed.

from @andydotxyz in #3121.

It may be worth noting that if Radius == Height/2 then the ends of the rectangle should be a semi-circle, but I don't think it is, so when adjusting the scaling calculations maybe check this too?

This statement was for me the wish to have a percentage alternative for radius.

How to solve this? The developer uses a container that changes the size depending on the windows' size. The included rectangle changes the size dynamically too.

The rectangle should look always the same, eg. semi-circle

In this case it would even be necessary to define the StrokeWidth as percentage to get an identical looking rectangle.
If you don't like the struct it could be named:

Radius float32 RadiusRelation float32 //RadiusDynamic RadiusPercent StrokeWidth float32 StrokeWidthRelation float32 // StrokeWidthDynamic StrokeWidthPercent

It could be possible to remove the percentage from the rectangle and to define a separate shape eg FlexRectagle, DynamicRectagle, where the Radius and SrokeWidth are always handled in percentage. This could be implemented later after this PR. I would prefer the solution with an own Shape-Type. It would be faster and would have less complexity.

Please let me know how to change the code.

I think you are putting to much smart into dumb canvas objects.
Something resizing the rectangle can set it's radius to be different if it is trying to maintain a specific shape.
Just like if someone wants a square - they have to ensure that the width and height match even if the container space is rectangular not square.

I don't think it's worth a more complicated API at this level to handle corner cases like that automatically. The canvasobjects are our primitive draw type - widgets using them are where the layout and render details are calculated.

OK, I will remove the percentage and the Radius will be Radius float32.

Refactor Radius done.

Bluebugs · 2023-01-08T18:24:06Z

I have created #3547 for this followup task. If that's good for you @Jacalz, we just need @andydotxyz review before we can merge this.

Jacalz · 2023-01-08T18:55:15Z

Sounds good to me. The only thing is that I think the point from @andydotxyz, about the tests not testing anything, still stands though. If we can't test it without the software render support, I personally think we can just remove the tests here but let's hear what Andy has to say first :)

andydotxyz · 2023-01-08T20:38:57Z

Sounds good to me. The only thing is that I think the point from @andydotxyz, about the tests not testing anything, still stands though. If we can't test it without the software render support, I personally think we can just remove the tests here but let's hear what Andy has to say first :)

Indeed there is a follow up task for the software renderer - but the markup renderer should be updated to understand this new field anyway. Not sure if we can use it in the test depending on circular imports

Jacalz · 2023-01-08T21:06:59Z

What I meant was, should we keep the tests that were added in this PR? They only test that a field is set?

andydotxyz · 2023-01-08T21:14:47Z

Indeed there is a follow up task for the software renderer - but the markup renderer should be updated to understand this new field anyway. Not sure if we can use it in the test depending on circular imports

Apologies, I meant to provide a little more detail but was afk:

diff --git a/test/markup_renderer.go b/test/markup_renderer.go
index a47b706f..28e5901e 100644
--- a/test/markup_renderer.go
+++ b/test/markup_renderer.go
@@ -332,6 +332,7 @@ func (r *markupRenderer) writeRectangle(rct *canvas.Rectangle, attrs map[string]
        r.setColorAttr(attrs, "fillColor", rct.FillColor)
        r.setColorAttr(attrs, "strokeColor", rct.StrokeColor)
        r.setFloatAttr(attrs, "strokeWidth", float64(rct.StrokeWidth))
+       r.setFloatAttr(attrs, "radius", float64(rct.Radius))
        r.writeTag("rectangle", true, attrs)
 }

I think that the tests there should be replaced by something that actually tests - such as:

	test.AssertObjectRendersToMarkup(t, "rounded_rect.xml", rect)

That way we are checking the state passes through - whereas the current tests are literally just checking that a field retains its value.

andydotxyz · 2023-01-08T21:17:56Z

For example the test:

func TestRectangle_Radius(t *testing.T) {
	rect := &canvas.Rectangle{
		FillColor:   color.NRGBA{R: 255, G: 200, B: 0, A: 180},
		StrokeColor: color.NRGBA{R: 255, G: 120, B: 0, A: 255},
		StrokeWidth: 2.0,
		Radius:      25}

	rect.Resize(fyne.NewSize(50, 50))
	test.AssertObjectRendersToMarkup(t, "rounded_rect.xml", rect)
}

renders

<canvas size="50x50">
	<content>
		<rectangle fillColor="rgba(255,200,0,180)" radius="25" size="50x50" strokeColor="rgba(255,120,0,255)" strokeWidth="2"/>
	</content>
</canvas>

andydotxyz · 2023-01-08T21:24:13Z

Just to clarify, I too am very happy with what this PR is and it's working well - if we get texts fixed up I'll be happy to approve too.

P.S. I know that many of the canvas objects are not very well tested - but mostly they were created before our test helpers!

andydotxyz · 2023-01-08T23:30:39Z

I added an example implementation of the software renderer to the issue @Bluebugs created in case anyone was curious / looking to add it.

renlite · 2023-01-09T09:58:47Z

For example the test:

func TestRectangle_Radius(t *testing.T) {
	rect := &canvas.Rectangle{
		FillColor:   color.NRGBA{R: 255, G: 200, B: 0, A: 180},
		StrokeColor: color.NRGBA{R: 255, G: 120, B: 0, A: 255},
		StrokeWidth: 2.0,
		Radius:      25}

	rect.Resize(fyne.NewSize(50, 50))
	test.AssertObjectRendersToMarkup(t, "rounded_rect.xml", rect)
}

I will replace the new test funcs (I created) in canvas/rectangle_test.go with this test func.

renlite · 2023-01-09T16:31:22Z

Go test fails but in testadata/failed/rounded_rect.xml is available? Radius is missing in XML.

PS C:\_project\renlite\fyne> go test .\canvas\rectangle_test.go
--- FAIL: TestRectangle_Radius (0.00s)
    test.go:115: Master not found at C:\_project\renlite\fyne\canvas\testdata\rounded_rect.xml. Markup written to C:\_project\renlite\fyne\canvas\testdata\failed\rounded_rect.xml might be used as master.
FAIL
FAIL    command-line-arguments  0.426s
FAIL

<canvas size="50x50">
	<content>
		<rectangle fillColor="rgba(255,200,0,180)" size="50x50" strokeColor="rgba(255,120,0,255)" strokeWidth="2"/>
	</content>
</canvas>

andydotxyz · 2023-01-09T16:49:50Z

Go test fails but in testadata/failed/rounded_rect.xml is available? Radius is missing in XML.

You need to copy the file from "testdata/failed" to "testdata" and commit it.

But for the radius to be present you need to include my adjustment to the markup writer:
#3539 (comment)

renlite · 2023-01-09T17:46:55Z

@andydotxyz Thank you for the infos. Test is working now.

andydotxyz

Amazing thanks!

Jacalz · 2023-01-10T07:28:31Z

Well done @renlite! 🙂

renlite · 2023-01-10T07:42:52Z

Thanks for the support by @Bluebugs and all the tests and good feedback by @andydotxyz @Jacalz @ErrorNoInternet.

renlite and others added 25 commits December 8, 2022 17:54

INIT Commit Round Rectangle

2cc826c

Merge branch 'fyne-io:develop' into develop

99e32b1

Light Example of Fyne Demo

2280d97

Merge branch 'develop' of https://github.com/renlite/fyne into develop

8f3619f

Color update in Light example

e8272d5

Time Measuring with new examples

37f926c

GPU rendered stroke rectangle (no round edges)

6f27034

Merge branch 'fyne-io:develop' into develop

c13e0de

Init commit GPU rendered round rect

088acba

Merge branch 'develop' of https://github.com/renlite/fyne into develop

6713a10

Merge branch 'fyne-io:develop' into develop

c7fd5ca

Clean up tests and debugging pixScale

3b163aa

Merge branch 'develop' of https://github.com/renlite/fyne into develop

6e6ac5c

Change outside color for debugging

b1b15aa

Debugging of frameSize and del unused param frame

486ae48

Debug frame_resolution in fragment shader

f21e5cf

scale implementation in fragment shader of rect

8269445

Split shaders for rect and round_rect

bd5248d

Further optimizations of round_rectangle.frag

9aa2246

Merge branch 'fyne-io:develop' into develop

2d58d13

Include Radius.Percent for RoundRectangle

916e111

Merge branch 'develop' of https://github.com/renlite/fyne into develop

d87b441

OpenGL ES implementation

d058400

Workaround unused attribute in rectangle_es.vert

9f2973f

Merge branch 'fyne-io:develop' into develop

151a0ed

Bluebugs requested changes Jan 5, 2023

View reviewed changes

renlite added 2 commits January 6, 2023 13:06

Refactoring to reuse code in gl/draw.go

d018916

Merge branch 'develop' of https://github.com/renlite/fyne into develop

1059425

andydotxyz requested changes Jan 6, 2023

View reviewed changes

Update rectangle_test.go Markup Test

e72914e

renlite dismissed stale reviews from Bluebugs and Jacalz via e72914e January 9, 2023 16:24

Merge branch 'fyne-io:develop' into develop

82e98f1

renlite requested a review from andydotxyz January 9, 2023 16:31

renlite added 2 commits January 9, 2023 18:44

Extend markup_renderer and copy rounded_rect.xml

c96940a

Merge branch 'develop' of https://github.com/renlite/fyne into develop

0bdbe90

renlite requested review from Bluebugs, Jacalz and andydotxyz and removed request for andydotxyz, Bluebugs and Jacalz January 9, 2023 21:52

Bluebugs approved these changes Jan 9, 2023

View reviewed changes

andydotxyz approved these changes Jan 9, 2023

View reviewed changes

andydotxyz merged commit 4e94498 into fyne-io:develop Jan 9, 2023

Uh oh!

Add OpenGL (ES)/GPU rendered rectangle and rectangle with rounded edges #3539

Add OpenGL (ES)/GPU rendered rectangle and rectangle with rounded edges #3539

Uh oh!

Conversation

renlite commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Uh oh!

Bluebugs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jacalz Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renlite Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andydotxyz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renlite Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bluebugs commented Jan 8, 2023

Uh oh!

Jacalz commented Jan 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andydotxyz commented Jan 8, 2023

Uh oh!

Jacalz commented Jan 8, 2023

Uh oh!

andydotxyz commented Jan 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andydotxyz commented Jan 8, 2023

Uh oh!

andydotxyz commented Jan 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andydotxyz commented Jan 8, 2023

Uh oh!

renlite commented Jan 9, 2023

Uh oh!

renlite commented Jan 9, 2023

Uh oh!

andydotxyz commented Jan 9, 2023

Uh oh!

renlite commented Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andydotxyz left a comment

Choose a reason for hiding this comment

renlite commented Jan 4, 2023 •

edited

Loading

Jacalz Jan 6, 2023 •

edited

Loading

renlite Jan 6, 2023 •

edited

Loading

coveralls commented Jan 5, 2023 •

edited

Loading

renlite Jan 6, 2023 •

edited

Loading

Jacalz commented Jan 8, 2023 •

edited

Loading

andydotxyz commented Jan 8, 2023 •

edited

Loading

andydotxyz commented Jan 8, 2023 •

edited

Loading

renlite commented Jan 9, 2023 •

edited

Loading