Add support for Image widget to handle a file URL or SVG #504

Cavallando · 2022-12-01T20:33:11Z

Hi tidbyt team!

I just got a tidbyt as a gift and I am absolutely loving how configurable it is and that a developer experience has been beautifully built out!

When I was playing around with writing my first app, I really wanted the ability to support SVG's. As I was writing that piece in my own app code I realized that this library could easily support that out of the box and further I added the ability to do URL as well so pixlet could be even easier than it already is at onboarding developers.

Hopefully it was ok that I opened this without being attached to an issue, I was trying to find Contributor guidelines but couldn't find much (please let me know if I missed something)

If there's any changes to syntax/style/etc please let me know, happy to change!

SVG +URL Example:

load("render.star", "render")

def main(config):
    return render.Root(
        child=render.Image(src="https://www.svgrepo.com/download/374110/svg.svg")
    )

betterengineering

Hey there! Thanks so much for the change! It appears there are two main features in this change:

Image by URL
SVG support

Image by URL is not something we can support at this time. We can support SVGs (and love the feature!), though we'd like to figure out the performance of SVGs before we do just to ensure we don't become dependent on a library that we cant support.

betterengineering · 2023-01-03T22:05:00Z

render/image.go


 	// register image formats
 	_ "image/jpeg"
 	_ "image/png"

 	"github.com/nfnt/resize"
+	"github.com/srwiley/oksvg"


Any reason for selecting this library? A quick search for golang svg provides github.com/ajstarks/svgo which appears to be much more popular, though I don't know the tradeoffs.

No specific reason for selecting this library! Full disclosure, I am somewhat new to the Go community so I just kinda googled and picked one.

I'll take a look at the library you suggested and circle back with tradeoffs between the two so we can discuss! (Although for other languages I tend to lean whichever has more community support/actively maintained so the one you linked might be the winner).

Alright so I found some more info about these packages:

github.com/ajstarks/svgo

Only supports SVG Spec 1.1

Does not include a rasterizer for converting from SVG -> Image out of the box. I found this GitHub issue where the maintainer recommended inkscape cli or a java tool which i doubt you all want to include as a dependency to pixlet

github.com/srwiley/oksvg

Supports SVG Spec 2.0 which is backwards compatible with SVG 1.1

It appears this library was built with rasterizing in mind given they wrote the rasterizer I am using `srwiley/rasterx

I'm assuming that writing a svg rasterizer is out of scope for pixlet (though I can try to take a stab at it if it's something you want!) But with that in mind in my opinion the library i'm currently using (srwiley/oksvg, srwiley/rasterx) seems like a little bit better fit.

Lmk what you think!

render/image.go

betterengineering · 2023-01-03T22:26:10Z

render/image.go

@@ -147,12 +159,66 @@ func (p *Image) InitFromImage(data []byte) error {
 	return nil
 }

-func (p *Image) Init() error {
-	err := p.InitFromWebP([]byte(p.Src))
+func (p *Image) InitFromSVG(data []byte) error {


It would be good to add a bench mark test here. I'm tempted to say we don't need it, but we risk some performance issues down the road that are hard to spot as I suspect SVGs can be rather expensive to rasterize.

Totally, great callout!

Do you mind sharing some insight on what a reasonable upper bound of that benchmark should be?

I don't currently. I'd probably run a go bench test on this method for all three image types and compare the results. I don't really have a strong sense of what reasonable would be. If it's twice as slow, that probably wouldn't be ok? But if SVGs are slightly slower, that's probably fine?

betterengineering

So sorry that this has been left open for so long. I am going to merge this, but will follow up with removing the InitFromURL method given it's unused. We also may have to remove this functionality if it becomes a performance issue.

netlify · 2023-07-31T12:48:27Z

✅ Deploy Preview for pixlet ready!

Name	Link
🔨 Latest commit	`2b8dcb4`
🔍 Latest deploy log	https://app.netlify.com/sites/pixlet/deploys/64c7ad99515c4b0008a161f6
😎 Deploy Preview	https://deploy-preview-504--pixlet.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Cavallando · 2023-07-31T15:10:53Z

@betterengineering ah no problem at all! Thank you for merging! And totally understand if it needs to be removed because of performance issues, lmk if I can help with that if it comes up!

rjison · 2023-11-17T04:28:06Z

So sorry that this has been left open for so long. I am going to merge this, but will follow up with removing the InitFromURL method given it's unused. We also may have to remove this functionality if it becomes a performance issue.

Any update on SVG support?
I tried this and it isn't working:

render.Image(src="https://media.kanjialive.com/kanji_strokes/ike_1.svg")

Cavallando added 10 commits December 1, 2022 10:54

init svg parser, rendering, needs options

375f434

Url image added, tests written

c87e11a

Removed unnecessary changes

4921590

Updated widgets docs

cf17a4f

Merge branch 'main' into main

4901b6c

Merge branch 'main' into main

cf66653

Fixed tests

d234350

Merge branch 'test'

78b06c1

Merge branch 'main' into main

9a7fe60

Merge branch 'main' into main

2a0c5be

Cavallando mentioned this pull request Jan 3, 2023

Feature: Plot - ChartType (line, scatter) #518

Merged

Merge branch 'main' into main

5ba6074

betterengineering suggested changes Jan 9, 2023

View reviewed changes

Cavallando added 2 commits February 7, 2023 09:41

resolved conflicts

bcb5793

removed image url feature

f56d451

betterengineering approved these changes Jul 31, 2023

View reviewed changes

Merge branch 'main' into main

2b8dcb4

betterengineering merged commit 33958cb into tidbyt:main Jul 31, 2023
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Image widget to handle a file URL or SVG #504

Add support for Image widget to handle a file URL or SVG #504

Cavallando commented Dec 1, 2022 •

edited

Loading

betterengineering left a comment

betterengineering Jan 3, 2023

Cavallando Jan 9, 2023

Cavallando Feb 7, 2023

betterengineering Jan 3, 2023

Cavallando Jan 9, 2023

betterengineering Feb 3, 2023

betterengineering left a comment

netlify bot commented Jul 31, 2023 •

edited

Loading

Cavallando commented Jul 31, 2023

rjison commented Nov 17, 2023

Add support for Image widget to handle a file URL or SVG #504

Add support for Image widget to handle a file URL or SVG #504

Conversation

Cavallando commented Dec 1, 2022 • edited Loading

betterengineering left a comment

Choose a reason for hiding this comment

betterengineering Jan 3, 2023

Choose a reason for hiding this comment

Cavallando Jan 9, 2023

Choose a reason for hiding this comment

Cavallando Feb 7, 2023

Choose a reason for hiding this comment

betterengineering Jan 3, 2023

Choose a reason for hiding this comment

Cavallando Jan 9, 2023

Choose a reason for hiding this comment

betterengineering Feb 3, 2023

Choose a reason for hiding this comment

betterengineering left a comment

Choose a reason for hiding this comment

netlify bot commented Jul 31, 2023 • edited Loading

✅ Deploy Preview for pixlet ready!

Cavallando commented Jul 31, 2023

rjison commented Nov 17, 2023

Cavallando commented Dec 1, 2022 •

edited

Loading

netlify bot commented Jul 31, 2023 •

edited

Loading