Skip to content

refactor pngwriter - #231

Merged
lstein merged 4 commits into
invoke-ai:mainfrom
bakkot:refactor-pngwriter-2
Aug 31, 2022
Merged

lstein merged 4 commits into
invoke-ai:mainfrom
bakkot:refactor-pngwriter-2

Conversation

@bakkot

@bakkot bakkot commented Aug 31, 2022

Copy link
Copy Markdown
Contributor

This makes the PngWriter class stateless, which makes it much, much easier to reason about. Now it just keeps track of the outdir, tells you what the next unused prefix is, and actually writes the files (and metadata) to disk. It doesn't know anything about seeds, upscaling, or auto-incrementing; it is the caller's responsibility to handle that sort of thing.

In passing, I've made it so that when the "save originals" option is selected for upscaling, the images are saved as "00001.seed.png" and "00001.seed.postprocessed.png", which I think is more useful than 01. (I don't actually have upscaling set up, so I haven't tested that part.)

I haven't made any changes to the actual metadata, but I'd like to do so. In particular, the fact that we're storing structured data as an unstructured string makes me unhappy; we should at least be JSON-ifying it. (Then the web UI could reasonably extract the metadata from an existing image, which would be handy - firing up the script for that is painful.) But I'll worry about that in a later PR.

Also, the metadata for grid images could be improved - it could store the data for each image individually, so that you could extract the information for just one image and re-run that. But again, something for a later PR.

@bakkot
bakkot force-pushed the refactor-pngwriter-2 branch from 23f0170 to d566ee0 Compare August 31, 2022 05:03
@lstein
lstein self-requested a review August 31, 2022 05:06

@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 is much simpler than the original, and I'm happy to merge it.

Just to explain the motivation for the original stateful implementation. I did not want to do an os.listdir() each time we needed to generate a new output file because there may be hundreds of files and on slow NFS-mounted systems this might introduce delays or system load. So the max filename was stored at initialization and then bumped up without doing subsequent listdirs(). However, this was just a theoretical concern and something to keep in mind in the event that performance issues appear later on.

@lstein
lstein merged commit 8a2b849 into invoke-ai:main Aug 31, 2022
@bakkot
bakkot deleted the refactor-pngwriter-2 branch August 31, 2022 14:39
@bakkot

bakkot commented Aug 31, 2022

Copy link
Copy Markdown
Contributor Author

That's a very reasonable motivation. Happily it's achieved by the design in this PR as well - this PR does exactly the same listdirs as before.

JPPhoto pushed a commit to JPPhoto/InvokeAI that referenced this pull request Sep 10, 2026
Custom fonts and a better Text tool properties pane
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.

3 participants