Enhancing skia further #357

tushar5526 · 2022-07-14T16:44:37Z

Transform

Settings

Attributes

Vertex

- translate - rotate

PS: apply_matrix has different structure than p5.js

- shear_x - shear_y

- background currently works only for colors, not images

- reset styles and matrix after each frame back to default

- stroke_cap() - stroke_join() - Moved stroke_cap and stroke_join to style class

- Bezier Vertex - Quadratic Vertex - Curve Vertex

- Begin Contour - End Contour

tushar5526 · 2022-07-16T20:48:42Z

Comparison of a few sketches. Skia followed by vispy, results look great and we are getting results similar to web canvas. Skia is really great.

skia_and_vispy.mp4

tushar5526 · 2022-07-16T21:44:21Z

Tried #298

particle_skia.mp4

particle_vispy.mp4

ziyaointl

Extra empty python file?

p5/core/transforms.py

p5/core/color.py

p5/core/vertex.py

ziyaointl · 2022-07-26T04:01:49Z

p5/core/vertex.py

@@ -309,8 +355,7 @@ def end_shape(mode=""):
    :type mode: str

    """
-    global is_bezier, is_curve, is_quadratic, is_contour
-    assert not is_contour, "begin_contour called without calling end_contour"


What was the reasoning behind removing this assert?

This assertion seems a bit wrong. The default value of is_contour is False so this assertion would raise an error even when no contour calls are made.

Also, is_contour indicates whether the current shape has a contour in it. So, we would have to use a different variable to indicate whether end_contour was called or not.

I think the default value of False actually does not trigger this assert, since not is_contour would evaluate to True.
I think the original is_contour had the function of denoting whether begin_contour has been called without end_contour being called. Without calling end_contour, it doesn't make sense to call end_shape, hence the original assert.
Under what condition did this assert trigger for you?

Ahh yes, you are right. This is what really happenedend_contour resets the value to False after begin_contour in the old code. I have removed that in the current code because we need to know whether is_contour was called or not to be passed onto renderer.end_shape. So now the assert is_contour will throw an error because the value was not set to False in end_contour

Ah I see. I think the old is_contour is still useful as a variable that indicates whether we are in a contour, thus providing us a way to nudge users to not call end_shape without calling end_contour after every begin_contour. I think the use case you've mentioned sounds like that a new variable named has_contour might be more appropriate. What do you think?

Yes, that sounds right. I have done that. Thanks!

p5/core/vertex.py

p5/sketch/Skia2DRenderer/renderer2d.py

ziyaointl · 2022-07-26T04:28:51Z

Great work! I left some comments, but majority of them are requests for clarification.

arihantparsoya · 2022-07-28T01:39:26Z

Looking great @tushar5526 . Have we done any performance comparison for this renderer? We can compare the FPS of skia renderer with old renderer and p5js as well.

I am sure this is great performance improvement. But it will be helpful for users to know these benchmarks.

arihantparsoya · 2022-07-28T01:45:42Z

Does skia renderer solve Retina display issues? I remember this was a major issue for many Mac users. #131

tushar5526 · 2022-07-28T05:54:13Z

Does skia renderer solve Retina display issues? I remember this was a major issue for many Mac users. #131

Yes, I configured glfw to support retina displays as well. But, I was not able to test it locally because I don't have a MAC with me. Mark has probably tested it locally so I think the pixel Density issues is not there.

tushar5526 · 2022-07-28T05:54:58Z

Looking great @tushar5526 . Have we done any performance comparison for this renderer? We can compare the FPS of skia renderer with old renderer and p5js as well.

I am sure this is great performance improvement. But it will be helpful for users to know these benchmarks.

I can use the current profiling to show the improvements in the release notes. I am open to any other ideas as to how we could measure the performance.

tushar5526 · 2022-07-31T20:07:47Z

Hey @ziyaointl I am thinking of starting with the new text and image APIs only after getting this one merged, followed by a release.

I think it will take some time for me to get the access to make a release. So I would be glad if you could make the release on pypi

ziyaointl · 2022-08-01T03:22:57Z

@parsoyaarihant I've been running vispy on a mac retina screen for a while now and it seems to work normally, so the issue probably has been fixed at some point in the past. I haven't been able to test skia on mac though due to a segfault. I'll probably have to install the skia library with debug symbols to find out what is causing that.

@tushar5526 Yes! Let me know when everything else in #360 has been finished, and I'll publish the 0.8.0 tag to pypi.

tushar5526 · 2022-08-01T06:14:37Z

This PR is good to go. @ziyaointl you can review and merge it :)

p5/core/vertex.py

…variable

tushar5526 added 10 commits July 14, 2022 22:06

Implemented and updated tests for

739f4a1

- translate - rotate

Implemented apply_matrix

5ed9832

PS: apply_matrix has different structure than p5.js

Added

08a96ad

- shear_x - shear_y

Implemented background

d35908a

- background currently works only for colors, not images

Bug fix

724de85

- reset styles and matrix after each frame back to default

Implemented

d444be5

- stroke_cap() - stroke_join() - Moved stroke_cap and stroke_join to style class

Some bug fixes after refactoring

9e59f62

Use set_stroke_join to set stroke join

285ed2d

Added stroke join and stroke cap for vispy's style class

f0c2f1a

Updated vertex APIs for skia

c939606

tushar5526 marked this pull request as draft July 14, 2022 17:20

tushar5526 added 4 commits July 16, 2022 01:02

vertex() and vertex(POINTS) shapes are implemented properly

ae5c1a2

vertex() API with all different shape kinds implemented

376d4df

Fixed

fb0ad5e

- Bezier Vertex - Quadratic Vertex - Curve Vertex

Fixes for

b5a3e0d

- Begin Contour - End Contour

tushar5526 marked this pull request as ready for review July 16, 2022 20:29

tushar5526 requested a review from ziyaointl July 16, 2022 20:29

Fixed Color bug

7661207

Added two sanity tests

f908f1d

ziyaointl reviewed Jul 26, 2022

View reviewed changes

tushar5526 added 7 commits July 26, 2022 18:33

Remove empty file

1e7e7f9

add todo for apply_matrix

90f9d79

Update the comment

fec71cd

remove unecessary check

a3a2e51

Add a TODO

836fc14

Add a TODO

0ad76d6

remove set stroke cap and join

3537235

Add relevant comments

dd98735

tushar5526 force-pushed the enhancing-skia-2 branch 2 times, most recently from 2843477 to 99a4aa2 Compare August 1, 2022 05:57

ziyaointl reviewed Aug 1, 2022

View reviewed changes

p5/core/vertex.py Outdated Show resolved Hide resolved

ziyaointl reviewed Aug 1, 2022

View reviewed changes

p5/core/vertex.py Outdated Show resolved Hide resolved

ziyaointl reviewed Aug 1, 2022

View reviewed changes

p5/core/vertex.py Outdated Show resolved Hide resolved

ziyaointl reviewed Aug 1, 2022

View reviewed changes

p5/core/vertex.py Outdated Show resolved Hide resolved

Add a check for proper contour calls

127f283

tushar5526 force-pushed the enhancing-skia-2 branch from 99a4aa2 to 127f283 Compare August 2, 2022 07:33

Added comment for future reference related to resetting contour flag …

2a07c44

…variable

ziyaointl merged commit bc0046f into p5py:master Aug 3, 2022

tushar5526 deleted the enhancing-skia-2 branch August 3, 2022 07:16

tushar5526 restored the enhancing-skia-2 branch August 3, 2022 07:16

tushar5526 mentioned this pull request Aug 7, 2022

particle system run very slowly #298

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancing skia further #357

Enhancing skia further #357

tushar5526 commented Jul 14, 2022 •

edited

Loading

tushar5526 commented Jul 16, 2022

tushar5526 commented Jul 16, 2022 •

edited

Loading

ziyaointl left a comment

ziyaointl Jul 26, 2022

tushar5526 Jul 26, 2022 •

edited

Loading

ziyaointl Jul 27, 2022

tushar5526 Jul 27, 2022 •

edited

Loading

ziyaointl Aug 1, 2022

tushar5526 Aug 1, 2022

ziyaointl commented Jul 26, 2022

arihantparsoya commented Jul 28, 2022

arihantparsoya commented Jul 28, 2022

tushar5526 commented Jul 28, 2022

tushar5526 commented Jul 28, 2022 •

edited

Loading

tushar5526 commented Jul 31, 2022

ziyaointl commented Aug 1, 2022

tushar5526 commented Aug 1, 2022

Enhancing skia further #357

Enhancing skia further #357

Conversation

tushar5526 commented Jul 14, 2022 • edited Loading

Transform

Settings

Attributes

Vertex

tushar5526 commented Jul 16, 2022

tushar5526 commented Jul 16, 2022 • edited Loading

ziyaointl left a comment

Choose a reason for hiding this comment

ziyaointl Jul 26, 2022

Choose a reason for hiding this comment

tushar5526 Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

ziyaointl Jul 27, 2022

Choose a reason for hiding this comment

tushar5526 Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

ziyaointl Aug 1, 2022

Choose a reason for hiding this comment

tushar5526 Aug 1, 2022

Choose a reason for hiding this comment

ziyaointl commented Jul 26, 2022

arihantparsoya commented Jul 28, 2022

arihantparsoya commented Jul 28, 2022

tushar5526 commented Jul 28, 2022

tushar5526 commented Jul 28, 2022 • edited Loading

tushar5526 commented Jul 31, 2022

ziyaointl commented Aug 1, 2022

tushar5526 commented Aug 1, 2022

tushar5526 commented Jul 14, 2022 •

edited

Loading

tushar5526 commented Jul 16, 2022 •

edited

Loading

tushar5526 Jul 26, 2022 •

edited

Loading

tushar5526 Jul 27, 2022 •

edited

Loading

tushar5526 commented Jul 28, 2022 •

edited

Loading