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

feat(image): avoid under image #94

Closed
rbalet opened this issue May 24, 2024 · 27 comments
Closed

feat(image): avoid under image #94

rbalet opened this issue May 24, 2024 · 27 comments

Comments

@rbalet
Copy link
Contributor

rbalet commented May 24, 2024

Description

Would it be doable to avoid rendering the middle of the qr-code ?

This would enable having gradient under the qr-code

image
@werthdavid
Copy link
Owner

I have an idea... will see when I can find time

@rbalet
Copy link
Contributor Author

rbalet commented May 27, 2024

@werthdavid nice! You can always let me know how you'd implement that and I can try on my own

@werthdavid
Copy link
Owner

So the idea is to use a globalCompositeOperation of "destination-out" with a dummy image.
See here: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/globalCompositeOperation?retiredLocale=de#changing_the_composite_operation
With this, you could remove the middle part. Should be quite easy.
BUT as always there's a catch.
It needs to be done inside kjua-svg. Which is not a problem generally speaking. I just thought that I could move all the code from kjua-svg directly to ngx-kjua which would also help avoiding those "optimization bailout" messages.
What do you say?

@rbalet
Copy link
Contributor Author

rbalet commented May 27, 2024

@werthdavid I see the idea here and it does seems to be a pretty good one. (Also, they didn't change the code for over 5 years )

So if I do understand it correctly, we'd need to transition first kjua-svg.

But maybe other ts frameworks folks could use a ts-kjua-svg library.
Wouldn't it be preferable to create a second library with typescript and import this one into the project?

I could get my hand on that today, what do you think?

@werthdavid
Copy link
Owner

yeah well, actually we don't even need ts-kjua-svg IMHO. If there is a proper build process, a JS library can be used without hassle, typescript definitions are there already. If you like, you are more than welcome to create a PR on kjua-svg (I'm the owner there as well).

@rbalet
Copy link
Contributor Author

rbalet commented May 27, 2024

@werthdavid Nice, I wasn't aware of it.

But that said, i'm not sure I understand why you wanted to import the kjua-svg code in ngx-kjua then, if you're able to fix the error at the source.

Can you create the required issue on the kjua-svg library and assign it to me ? (Or mention me in the issue).

I'll have a look this afternoone.

That said, I may need more time that you for the solution you've proposed with compositionOperation, as I'm actually specialised in Angular, but not the rest '^^ (But always eager to learn)

@werthdavid
Copy link
Owner

I wanted to import the code because it would be the quicker (but dirty) solution. I forked kjua-svg from the original "kjua" only to have svg support but the whole project for me was only there to have the angular component. the original "kjua" now has svg support as well btw.

so the clean way would be to fix kjua-svg, no doubt.

If you want to implement the feature, I'd do the following:

@rbalet
Copy link
Contributor Author

rbalet commented May 27, 2024

@werthdavid Ah I see, so, the must would be to use kjua directly, instead of kjua-svg and as we cannot edit kjua and it seems to not being maintained anymore, it could be worth to import it into the ngx-kjua project directly.

So I'll implement it directly in ngx-kjua.

Then, if we wish to migrate it to the official kjua library, we could, and ask them to merge it.
But that we we could move forward with the ngx-kjua does that makes sense ? (Sorry just wanna be sure '^^)

@werthdavid
Copy link
Owner

Careful! The original kjua (which is lrsjng/kjua) is not maintained anymore. My fork (werthdavid/kjua) is published as "kjua-svg" as I added svg support (later, lrsjng added svg support to the original lrsjng/kjua as well). I didn't switch back to the original kjua and kept my fork as I added the "Image as Code" option there. The logic of this option can be "safely inherited" (as I call "copy&paste") to implement this feature.

So if I would do it, I would potentially:

  • copy the code from kjua-svg (werthdavid/kjua) to this repo
  • make the changes as stated in the answer above
    (to make the changes, it is NOT actually necessary to copy the sources to THIS repo! But since the original kjua and kjua-svg have a strage build process we get optimization-warnings from angular when it is used here. which has always been going on my nerves... so at this point when I would touch kjua-svg again I would copy the sources here as we have a proper build process here) hope my intention gets clear!

OR:

  • make the changes in werthdavid/kjua, bump the version, use the version here and we're good to go.
    (but then there would still be the optimization-warnings. but they are totally unrelated to this issue!)

does that make sense?

@rbalet
Copy link
Contributor Author

rbalet commented May 27, 2024

@werthdavid Yeh, that warning also did get on my nerves.

I think I've understood everything, I'll start right away, thx for the clarification

@rbalet
Copy link
Contributor Author

rbalet commented May 27, 2024

@werthdavid I've done the .ts implementation, we're just missing this part, which I'll do (or you can do preferably), once this one have already been accepted

ToDo

@werthdavid
Copy link
Owner

thank you @rbalet ! I will take a look!

@werthdavid
Copy link
Owner

So the PoC works, but unfortunately only for canvas and img, my idea with "globalCompositeOperation" doesn't work with svg... is that a limitation you could live with?

@rbalet
Copy link
Contributor Author

rbalet commented May 29, 2024

@werthdavid I'd say it is already better than nothing 😍.

If you wish, I could add an implementation that throw an error when svf and glovalCompositieOperstion are used.

And thx for the implementation, really appreciated

@werthdavid
Copy link
Owner

I will merge the canvas/image option now and throw an error for svg. no big deal. I have an idea how it COULD work with svg but don't really have time these days.

@rbalet
Copy link
Contributor Author

rbalet commented May 29, 2024

@werthdavid Np, it's already pretty good if you can push that one.

Here, I've create an issue for the svg

@werthdavid
Copy link
Owner

sorry for the delay, had some issues with the unit-test, the option is now available. please let me know if everything works with 18.1.0

@rbalet
Copy link
Contributor Author

rbalet commented Jun 3, 2024

@werthdavid No problem, thx a lot for doing it either way.

I do get the following error
image

That I've fixed right away in that pull request

@rbalet
Copy link
Contributor Author

rbalet commented Jun 3, 2024

@werthdavid Can you show me where to add the

Also, somehow we now get this error while importing (Funnily enough, this does work inside the ngx-kjua test library..
Maybe you have an Idea

image

@werthdavid
Copy link
Owner

add the what? 🙈
how can I reproduce this error?

@rbalet
Copy link
Contributor Author

rbalet commented Jun 10, 2024

@werthdavid so

Step to reproduce

in the kjua project

  1. run npm i ngx-kjua@latest
  2. use the ngxKjuaComponent > import { NgxKjuaComponent } from "ngx-kjua";
  3. You'll have the error.

Somehow it seems like it is wrongly complied

I have seen the following discussion : https://stackoverflow.com/questions/43946738/webpack-compile-error-typeerror-webpack-imported-module-1-is-not-a-funct

It seems it could be due to the import

I may think that this can be the problem.
image

But I don't know how to import this one in a other manner..

@rbalet
Copy link
Contributor Author

rbalet commented Jun 10, 2024

@werthdavid Hey, I've tested the following fix. #99

I've let the preserveSymlink in the angular.json file so that you can try with the npm link.

For me, it seems to have fixed the issue.

Can you please try out ?

@Sergiobop
Copy link

Hi @rbalet , is this the same issue i described here? #98

You talk about the same stringToBytes line

Thanks

@rbalet
Copy link
Contributor Author

rbalet commented Jun 11, 2024

@Sergiobop Exactly

@rbalet
Copy link
Contributor Author

rbalet commented Jun 12, 2024

@werthdavid I confirm that this is working.

That said, this does not resolve this issue as the image is in the qr-code, and not cut out of it

(But still looks great )
image

@werthdavid
Copy link
Owner

What settings did you provide for given code?

@rbalet
Copy link
Contributor Author

rbalet commented Jun 12, 2024

@werthdavid Forget what I've said.

Closing the issue as it works juste like it should.
Thx again for your work

@rbalet rbalet closed this as completed Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants