Skip to content

webui: hide gfpgan part if not installed - #187

Merged
lstein merged 1 commit into
invoke-ai:mainfrom
bakkot:webui-upscalers-optional
Aug 29, 2022
Merged

lstein merged 1 commit into
invoke-ai:mainfrom
bakkot:webui-upscalers-optional

Conversation

@bakkot

@bakkot bakkot commented Aug 29, 2022

Copy link
Copy Markdown
Contributor

This has the first two commits from #186 just to avoid merge conflicts. Third commit is the actual logic.

This hides the GFPGAN part of the web UI (and suppresses error messages in the server console) if the GFPGAN model isn't found. I actually don't have GFPGAN installed and so can't test that it still displays when the model actually is installed, but it should.

@lstein lstein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are LF vs CRLF issues in the changed files, making it look like the entire files are updated. @bakkot could you fix? Otherwise, everything seems in order.

@bakkot

bakkot commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

I think there are LF vs CRLF issues in the changed files

@lstein That's intentional; see #186. Other files were all using LF, so I switched these to match (in separate commits).

@lstein

lstein commented Aug 29, 2022

Copy link
Copy Markdown
Collaborator

I think there are LF vs CRLF issues in the changed files
@lstein That's intentional; see #186. Other files were all using LF, so I switched these to match (in separate commits).
Thanks for that. I misunderstood.

@lstein lstein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

@bakkot
bakkot force-pushed the webui-upscalers-optional branch 3 times, most recently from dce0d2b to e34d3ea Compare August 29, 2022 17:06

@blessedcoolant blessedcoolant left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good. I'd make 2x the default value for the Upscaling Level dropdown. Currently it is not set to anything on the UI. Rest is good to go.

@bakkot

bakkot commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

@blessedcoolant I would think upscaling should be opt-in, even if you have GFPGAN set up? So blank (i.e. no upscaling) seems like the right default. Anyway this PR doesn't change the default.

@blessedcoolant

blessedcoolant commented Aug 29, 2022

Copy link
Copy Markdown
Collaborator

@blessedcoolant I would think upscaling should be opt-in, even if you have GFPGAN set up? So blank (i.e. no upscaling) seems like the right default. Anyway this PR doesn't change the default.

I guess. But seeing an unfilled element in the UI is never good. I would ideally prefer there to be an option that just says 'None' or something. Not a big issue though.

I'm gonna do a full fledged UI when we are done with the big features and we can explore the UI options then.

@bakkot

bakkot commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

I would ideally prefer there to be an option that just says 'None' or something.

Looks like #192 actually does that.

@TesseractCat TesseractCat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice way to pass config info. The #gfpgan element should be hidden by default, to prevent elements moving around when loading the page without GFPGAN installed.

@bakkot

bakkot commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

@TesseractCat well, either elements move around during load when it is installed or they move around when it isn't; you can't really avoid that. In practice, when running locally it's close enough to instantaneous that you're not going to see it happen. So I'm going to leave it as-is.

@bakkot
bakkot force-pushed the webui-upscalers-optional branch from e34d3ea to a7a62d3 Compare August 29, 2022 23:28
@lstein

lstein commented Aug 29, 2022

Copy link
Copy Markdown
Collaborator

I'm going to go ahead and merge. If the aesthetics get bothersome we can always tweak later.

@lstein
lstein merged commit e7658b9 into invoke-ai:main Aug 29, 2022
@bakkot
bakkot deleted the webui-upscalers-optional branch August 29, 2022 23:29
austinbrown34 pushed a commit to cognidesign/InvokeAI that referenced this pull request Dec 30, 2022
JPPhoto pushed a commit to JPPhoto/InvokeAI that referenced this pull request Sep 1, 2026
…umerically

Runs every mode on the stock conv3d and then on the conv2d decomposition
in one process and reports the drift between the two paths, so a partial
revert of the HIP >= 7.2 retirement (invoke-ai#187) can be checked for the
allocator-state corruption that motivated it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S4B5exWsbC2z2Uu2tA167A
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.

4 participants