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

[background image] Add dialog for background image #1789

Closed
wants to merge 5 commits into from

Conversation

ollimeier
Copy link
Collaborator

Fixes #1787

Work in progress -> therefore draft PR.

I had to use the other branch as the base branch (not main), to have the selection code included.

@ollimeier
Copy link
Collaborator Author

@justvanrossum I found a solution to make the color apply on a png. This is based on the idea to make every white pixel transparent. Reference: https://stackoverflow.com/questions/30438524/algorithm-to-make-image-white-and-transparent

@justvanrossum
Copy link
Collaborator

It doesn't work for me: I see the whole image as a solid rectangle in the selected color.

@justvanrossum
Copy link
Collaborator

This should be rebased on main somehow.

@ollimeier ollimeier force-pushed the issue-1787-add-dialog-for-background-image branch from 7761721 to dc21474 Compare November 15, 2024 13:53
@ollimeier ollimeier force-pushed the issue-1787-add-dialog-for-background-image branch from dc21474 to f0ddab9 Compare November 15, 2024 23:53
@ollimeier
Copy link
Collaborator Author

It doesn't work for me: I see the whole image as a solid rectangle in the selected color.

Does it work now? I made some changes. Search for function white2transparent(img). It's definitely not the right solution, but a demo of the idea 'convert white pixel to transparent pixel'.
I ask myself in general whether this is a good idea and, if so, whether it should be done in the backend or if in the frontend?
(I do not work today, but it bothered me and I am currently at the airport, where I had some time to look into it)

@ollimeier
Copy link
Collaborator Author

@justvanrossum I am also wondering if we should wast our time with the color image issue. Would a user really use that kind of feature? I think the opacity feature is the more important one.
Having said that, I am wondering if setting the opacity for each image individually is the right thing to do or if we prefer to include such settings more globally in our applications settings? So, that all images have the same opacity, or at least a global default opacity and if some wants to adjust a specific one, they can do this as well?

@justvanrossum
Copy link
Collaborator

It does work now. But I notice that this colorization is very binary: any grayscale values are gone. So that's definitely not desired.

Let's for now not do colorization, and only provide the opacity slider.

About having a global setting: maybe. Let's start with the individual opacity slider.

(There may be better options with the Canvas filter property, but this isn't supported in Safari.)

@justvanrossum
Copy link
Collaborator

I have found the solution that doesn't involve pixel hacking. To properly support opacity, we unfortunately need to do the colorization into a temporary canvas. I got the colorization working efficiently (using caching) here: #1815. Will work on the color picker UI in the selection info panel.

@ollimeier
Copy link
Collaborator Author

I have found the solution that doesn't involve pixel hacking. To properly support opacity, we unfortunately need to do the colorization into a temporary canvas. I got the colorization working efficiently (using caching) here: #1815. Will work on the color picker UI in the selection info panel.

This is awesome. Thank you very much.

@ollimeier ollimeier deleted the issue-1787-add-dialog-for-background-image branch November 20, 2024 00:39
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

Successfully merging this pull request may close these issues.

[background image] Add a dialog for image properties
2 participants