-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[WebUI] Fix 'Use All' Params not Respecting Hi-Res Fix #2840
[WebUI] Fix 'Use All' Params not Respecting Hi-Res Fix #2840
Conversation
6f52c4d
to
4882710
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for redoing this PR. I've requested two changes.
invokeai/frontend/src/features/parameters/store/postprocessingSlice.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, pending the changes requested by @psychedelicious
d81d39b
to
79606f8
Compare
79606f8
to
f207647
Compare
@blhook is this ready for review? Sorry if I missed a notification. |
@psychedelicious , yes, it should be ready. I probably should have made a specific comment for it after the push (which was not too long after my replies to the conversation). |
Ah, good. I'll do my review this evening. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good, thank @blhook !
This is a different source/base branch from #2823 but is otherwise the same content.
yarn build
was ran on this clean branch.What was the problem/requirement? (What/Why)
As part of a change in 2.3.0, the high resolution fix was no longer being applied when 'Use all' was selected. This effectively meant that users had to manually analyze images to ensure that the parameters were set to match.
Additionally, and never actually working, Upscaling and Face Restoration parameters were also not pulling through with the action, causing a similar usability issue.See: #2823 (comment)What was the solution? (How)
This change adds a new reducer to the
postprocessingSlice
file, mimicking thegenerationSlice
reducer to assign all parameters appropriate for the post processing options. This reducer assigns:txt2img
, sinceimg2img
hi-res was removed previouslyUpscaling's toggle button, scale, denoising strength, and upscale strengthFace Restoration's toggle button, type, strength, and fidelity (if present/applicable)Minor
endOfLine: 'crlf'
to prettier's config to prevent all files from being checked out on Windows due to difference of line endings (and git not picking up those changes as modifications, causing ghost modified files from Git)Revision 2:
Revision 3:
hires_fix
not present (assume false)Out of Scope
How were these changes tested?
yarn dev
=> Server started successfullyyarn build
=> SuccessNotes
As with
generationSlice
, this code assumesaction.payload.image
is valid and doesn't do a formal check on it to ensure it is valid.