Skip to content

Conversation

wandbrandon
Copy link

@wandbrandon wandbrandon commented Jan 30, 2025

@newfla Draft of api changes. Does not support all the features of sdcpp just yet, and I don't expect a merge or anything, just curious on your opinion of the api.

notable changes:

  • ModelConfig and Txt2ImgConfig are separate builders to more closely resemble the original binding
  • ModelCtx struct to keep track of current sd_ctx and ability to call txt2img on this struct directly
  • free params immediately is defaulted to false to support holding the model in memory (needs some error checking for sure)
  • add_lora_model is added to txt2img config builder to add loras to prompt as part of builder paradigm
  • saves images to image crate type instead of directly to file, incase we want to use image in memory without saving (not tested, not 100% sure how sdcpp handles the image buffer)

Let me know what you think! Apologies if there are any glaring issues.

@newfla
Copy link
Owner

newfla commented Feb 3, 2025

Hi @wandbrandon thank you for your contribution.
I think we ended up doing similar things by splitting model and txt2img params into diffrent structs following more strictly the original api.
The only thing that I would do differently is the model_ctx field: I think wrapping the pointer into an Option is more idiomatic code:

  • None --> ctx not initialized
  • Some(ctx) --> ctx initilaized and ready to be used

I'm very interested in the image crate support: I would be very happy to have your contribution to eliminate the dependency to stb_image dependency

@wandbrandon
Copy link
Author

wandbrandon commented Feb 3, 2025

Hi @newfla, yep I noticed our apis were similar, which is great. Seems like the issue can be closed unless you'd like to implement the image crate support as part of it. Thank you for your help!

Regarding the support for image crate, it seems to be working just fine. Tested on some generations and it works great.

Im not sure if sdcpp's save image to file supports metadata baked within the png files like other frontends do, (comfyui, a11111, etc.). If it did that could be useful. That's the only reason why I would keep the stb_image dependency. Otherwise, the image buffer conversion highlighted here works well.

I was going to start on img2img

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.

2 participants