Closed pngwn closed 6 months ago
• | Name | Status | URL |
---|---|---|---|
Spaces | ready! | Spaces preview | |
Website | ready! | Website preview | |
Storybook | ready! | Storybook preview | |
Visual tests | all good! | Build review | |
:unicorn: | Changes | detected! | Details |
:notebook: | Notebooks | matching! |
Install Gradio from this PR
pip install https://gradio-builds.s3.amazonaws.com/0b5240eafd96b4138875c2c37a2a02cb54c16807/gradio-4.4.1-py3-none-any.whl
Install Gradio Python Client from this PR
pip install "gradio-client @ git+https://github.com/gradio-app/gradio@0b5240eafd96b4138875c2c37a2a02cb54c16807#subdirectory=client/python"
Package | Version |
---|---|
@gradio/app |
minor |
@gradio/atoms |
minor |
@gradio/icons |
minor |
@gradio/image |
minor |
@gradio/imageeditor |
minor |
@gradio/preview |
minor |
@gradio/statustracker |
minor |
@gradio/upload |
minor |
gradio |
minor |
⚠️ Warning invalid changelog entry.
Changelog entry must be either a paragraph or a paragraph followed by a list:
<type>: <description>
Or
<type>:<description> - <change-one> - <change-two> - <change-three>
If you wish to add a more detailed description, please created a
highlight
entry instead.
⚠️ The changeset file for this pull request has been modified manually, so the changeset generation bot has been disabled. To go back into automatic mode, delete the changeset file.
@pngwn I'm happy to review this, but it looks like the frontend code may have broken as a result of the v4 changes, and it can't build at the moment.
Reviewed the backend, looks very good. I'm excited about the Brush and Eraser classes, and will try to recreate some of the old demos (like the sketchpad demo) with this.
Got a few v4 bugs to fix but I'll be picking this back up shortly and I'll take care of the issues and finish it off.
hmm
Amazing work @pngwn! I just gave the image editor a quick spin and noticed a few potential bugs / UI suggestions:
When you're starting out, seeing all 7 buttons grouped together can be a bit overwhelming. I was thinking we could show only the 1st row of upload/webcam/paste buttons initially, and then show the others when you've provided an image. But I don't think that would work since you can sketch on a blank canvas without having provided any image. What if we moved the 2nd row of buttons to the side, in a vertical column underneath the "X" button. I feel like that might be a bit more intuitive than the current setup. (cc @hannahblair for your thoughts!)
Clicking the webcam icon opens a full-screen webcam capture. I assume not intentional?
The crop works beautifully. But clicking the rotate image icon has no effect.
Similarly, I can't get sketching or erasing working for me. Clicking the palette icon or the gray button has no effect and does not allow me to sketch on the base image
demo/image_editor/run.py
and clicking the "Run" button does not do anything for me. I see this error in the console: Uncaught Error: Could not create blob
We could just start off with the brush selected, that way we have fewer buttons. We could even start off with nothing selected but again i think that is worse UX even though it might be OK visually. I don't really think users will be overwhelmed by 6 buttons though, most image editors have dozens on display at all times.
It is intentional but given your response, I'll tweak that.
I am going to remove rotate for now and implement later. Its complex (not the rotate, the UX).
I'll fix sketching / erasing. There is a bug atm (well 2). SO you have to crate a new layer and change the size of the brush for it to work. Should be sorted shortly.
Probably need to handle empty layers more gracefully. Will check that.
also:
Good stuff @pngwn! I was able to get it running locally just fine. Here are the issues that I noticed:
The "X" icon to clear the image has no effect
I don't think the image data is being preprocessed correctly. Neither of these two demos work (no image output is shown when you click submit):
import gradio as gr
demo = gr.Interface(lambda x:x, gr.ImageEditor(), gr.ImageEditor())
if __name__ == "__main__":
demo.launch()
import gradio as gr
demo = gr.Interface(lambda x:x["composite"], gr.ImageEditor(), gr.Image())
if __name__ == "__main__":
demo.launch()
Some UI improvements would be nice (though we can do these in a separate PR afterwards):
Cropping, brushing, layers, erasing, undo, and redo all work super well!
@abidlabs I've fixed all of those issues now I think. Also fixed the change
and input
events.
The issue with interface wasn't the preprocessing but the static variant.
Incredible work. Playing around with this, the few points of friction I hit:
Brush sizes show weird numbers
That's pretty hilarious. Will remove the default size swatches for now. Not sure how useful they are.
I've tightened up the buttons a little now too. I have some ideas about keeping them one on line but will discuss first + try in another PR.
Everything works great!! Awesome work @pngwn
A few notes:
For another PR, it'd be good to have an empty state before a file has been uploaded - e.g. the file upload box is shown on other components when they're empty
Super tiny nit, I'd love a small bit of spacing around the eyedropper icon
The whole colour picker component is so nice!
I think I broke the button background. Will check that.
I'll take a look at those other issues too. Dark mode looks a bit janky. Will fix.
Like the tightened buttons a lot. Noticed some small things:
Can we put a "drop image here" text similar to gr.Image()
when you're on the uploading step
After you upload an image, would it make sense for it to automatically go to the next step of cropping?
When you click on the paintbrush step, its not clear what the paintbrush color is before you start drawing. Can we make the color of the brush radius selector match the paintbrush color?
The ImageEditor doesn't have a progress indicator, e.g. try with:
import gradio as gr
demo = gr.Interface(lambda x:x, gr.ImageEditor(), gr.ImageEditor())
if __name__ == "__main__":
demo.launch()
just fyi removed some console logs so that we could run the static checks without linter complaining
thanks for taking the time to add jsdocs btw!
@abidlabs
crop_size
is set by the author it will automatically go into crop though.For (5), yes when it is an output
@abidlabs @aliabid94 @hannahblair
Thanks for the reviews!
I think i've addressed everything.
@hannahblair I added some edge collisions to fix the issues on mobile (although mobile needs looking at on its own) and I tweaked darkmode so the color picker looks much nicer. I've also fixed the icon button (i did break them).
@aliabid94 I've cleaned up the UX regarding controls and remove those brush size related issues. I've also added a 'pain' cursor that matches the brush size as a preview for both erasing and drawing. Everything should be working on 3.8 now too (thanks @abidlabs ).
@abidlabs I've added a message for the blank state to give some pointers. We can probably improve that in the future. I've also fixed the statustracker and added the color preview on the circle.
The main new thing is you can now pass 'crop constraints' via crop_size
and this will force the crop to match that ratio on the frontend and be resized fully on the backend. You can pass either a fixed width and height or a string ratio "1:1"
.
Doing some testing right now. Generally everything is working great. Just very tiny nits that I've found so far:
I would have expected clicking the "Upload an image or select..." box to open the upload file dialog like it does for our other components.
When you use an ImageEditor as an input and trigger the event (e.g. click a submit button), it takes quite a while before anything actually happens.
E.g. running
import gradio as gr
gr.Interface(lambda x:x, "imageeditor", "imageeditor").launch()
I have to wait ~5 seconds before even just seeing the progress status tracker on the output component.
import gradio as gr
gr.Interface(lambda x:x, "imageeditor", "imageeditor").launch()
if you upload an image and then press the gray "Clear" button in the Interface, nothing happens.
sources=[]
, then the placeholder text should be changed as to not mention uploading an image:If sources=[]
, should we also remove the transforms
by default? At the very least, we should set the initial state of the ImageEditor component to start on the sketch tool, not on the cropping tool, as that is confusing.
We may want to add a parameter in Brush
to disable custom color selection. E.g. if you want to constrain users to only be able to draw a single-color mask.
On narrow screens, the color selection is cut off. At least we can increase its z index so that the color wheel is still visible.
I'll take a look at 1,2,3,3 and 5.
I don't think we should remove transforms entirely but I'll change the default.
Should already be possible by setting color_mode
in Brush
to "fixed"
.
I thought I fixed this, will double check. It might need a resize to take effect.
Okay, made a few changes.
I couldn't add click to upload yet due to how things are put together, we can do that later if it is really needed but feels off for an image editor imo.
Clear
buttonsources=[]
the draw tool will be selected.None
cases get a default brush), so eraser
and brush
can now accept a boolean too.And we are done!
Thanks @abidlabs @hannahblair @freddyaboulton @aliabid94 for the help with various bits and pieces, the PRs, and the detailed reviews + tested. Very much appreciated!
Hey all,
thanks for the wonderful work. I am still wondering about a few things though
user facing
This is the new
ImageEditor
, which is a replacement for 3.x'sImage
withtool=...
. I have linked the issues that it fixes below (both bugs and enhancements), so i wont go over every detail but the high level is as follows:This component is a simple + streamlined image editor that handles basic image manipulations. It should be at least equivalent to the old Image with various
tools
set but less bad and more good. Additional features include:Layers
Background
upload
will upload a file from the users computerwebcam
will open a webcam capture window which will be set as the bg when it is captured.paste
will paste from clipboardDraw
Erase
Crop
Undo
Redo
Performance
technical
The new Image Editor is built on top of pixi.js, a javascript library for creating 2d webgl experiences. It is an exceptional library if a little confusing but it sits at an almost perfect level of abstraction. High level enough so you don't need to think too much about the inner workings, low level enough that you can dive in when you need to. It is not a sketch library in any way but has all of the correct primitives. It is also really fast.
The image editor has a bunch of code but its a lot less than I thought it would need to be. The image editor has been designed to be flexible and maintainable, it should be relatively easy to add new functionality without needing to touch the whole thing. For example, modification to the crop tool, should only require changes to the cropping part of the code.
The editor uses the command pattern which is a little complex but incredibly powerful. Every editing action is a 'command'. A command knows how to perform itself and how to undo itself. We have a command manager that takes in every command, and adds it to its history. When we want a new feature, we create a new command that is dedicated to just that thing.
crop
is a command,draw
is a command, 'add bg image` is a command.When we want to undo something we ask the command manager to undo it, it will then call the current history node's
undo
method and move a point back. And repeat.When we want to redo something it will call the 'next' commands
execute
method, and move the history point forwards. The history is implement as a doubly linked list so it is simple to traverse and very difficult to skip nodes.The 'command' interfaces look like this:
Each command can decide how to implement all of these methods, it doesn't matter as long as they 'work'. Only
execute
andundo
are required.The reason
start
continue
andstop
exist is best explained with an example:Lets take drawing. A single drawing operation is one line. So
execute
should draw an entire line onto the canvas (for example when we are 'redoing' a line). However, we can only call execute when the line has finished drawing because that will get added to the history as a single entity. Since this is an image editor we need real time updates. With this in mind, i addedstart
,continue
, andstop
as optional methods that can be used to provide realtime updates to the canvas, while still giving us the option to undo/redo later.In the case of drawing:
start
when a user presses their mouse down. We set up some initial textures and snapshot the initial state.continue
. here we are interpolating between x/y points (to ensure a smooth line), drawing to the texture we set up before and rendering it.stop
. In stop we perform some final clean up and reset the textures. We reuse the textures for performance reasons, so this clean up is important.After realtime drawing
execute
is basically a no-op (it always get called when we pass it to the command manager). However if no realtime drawing has been completed then it will draw the full line.The reason it is important that the command manager is able to handle these realtime updates is because all drawing operations then just live in this one ovject which makes maintenance much easier. Since everything is passed into these commands as arguments, unit testing is also straight forward.
The command pattern is actually really nice for this use case and allows us to easily add complex features without drowning in a sea of complexity. It also unlock other possible features in the future (really fancy ones like macros/ batch processing).
jsdoc
I have made extensive use of jsdoc annotations throughout the code in the hope that it offers a little help for us all when navigating this component in the future.
This is a practice that I think we should experiment with across the rest of the code base (frontend). I personally find it very helpful, both when writing and reading code because it makes me think about my interfaces + what they are actually doing.
Closes #5645 Closes #5206 Closes #5152 Closes #5055 Closes #4931 Closes #4907 Closes #4842 Closes #4677 Closes #4653 Closes #4492 Closes #4413 Closes #4290 Closes #4252 Closes #4159 Closes #4120 Closes #4011 Closes #3810 Closes #3623 Closes #3535 Closes #3472 Closes #3305 Closes #3280 Closes #3138 Closes #3110 Closes #2649 Closes #2425 Closes #2314 Closes #1591 Closes #466
🎯 PRs Should Target Issues
Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.
Not adhering to this guideline will result in the PR being closed.
Tests
PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests:
bash scripts/run_all_tests.sh
You may need to run the linters:
bash scripts/format_backend.sh
andbash scripts/format_frontend.sh