search

ltdrdata / ComfyUI-Impact-Pack

Custom nodes pack for ComfyUI This custom node helps to conveniently enhance images through Detector, Detailer, Upscaler, Pipe, and more.
GNU General Public License v3.0
1.89k stars 183 forks source link

New to open source but have added functionality #528

Open shanevcantwell opened 7 months ago

shanevcantwell commented 7 months ago

Hi!

I modified modules\impact\impact_pack.py class IterativeLatentUpscale to add to the existing functionality the ability to step evenly by total pixel count re-calculating sides logarithmically (although I'm now merging the geometric mode from 2 days ago so it'll be a bit longer to finish). The thought is that since pixel count rises exponentially as sides grow linearly, this way scales evenly by pixel count to the upscale amount reducing the number of steps logarithmically. I had an inspiration that this might lead to cleaner ksample resampling with size, but thinking about it now I'm not so sure it would do anything but make VRAM rise linearly instead of exponentially between steps which probably isn't very useful. It could at least also reduce number the number of steps which would give a possible performance boost especially when tiling.

At any rate I'm going to play with it locally since I already implemented it. Does anybody have any comment on whether this would be the least bit useful functionality for them?

Let me know - I'd like to contribute to ComfyUI but have never contributed to any open source code before. What process I would follow before committing changes? Is there an owner gateway system i.e. somebody to grant permission? I followed all the implied naming and logic conventions with efficient and consistent logic wrt existing code, and made everything very readable without_using_overly_huge_variable_names.

Thanks to anybody who has comments/answers!

(also it looks like the new geometric functionality might be doing the same thing in a different way anyway...)

shanevcantwell commented 7 months ago

Yep, this is literally the same functionality checked in the day I started it. Except it doesn't take account the changes to the side lengths.

ltdrdata commented 7 months ago

To contribute, fork the repository you want to contribute to, make changes and commit them to your forked repository, then submit a Pull Request. The maintainer of the original repository will review your changes and decide whether to merge them.

ltdrdata commented 7 months ago

During the review of this https://github.com/ltdrdata/ComfyUI-Impact-Pack/pull/523, I've noticed that the existing simple mode indeed produces images closer to the original when compared to the new geometric mode. However, each mode has its own advantages and disadvantages, so I still believe there is merit in maintaining both modes. As a result, I've modified it to incorporate the modes.

If you wish to propose a new scaling mode, you could contribute by adding it as an additional mode.

shanevcantwell commented 7 months ago

Nice! I'm pretty close to finishing it up but hitting the 90-90 rule of course. I forked tho and I think correctly. Do I post the whole distro or just changed files?

On Thu, Mar 21, 2024 at 12:38 AM Dr.Lt.Data @.***> wrote:

During the review of this #523 https://github.com/ltdrdata/ComfyUI-Impact-Pack/pull/523, I've noticed that the existing simple mode indeed produces images closer to the original when compared to the new geometric mode. However, each mode has its own advantages and disadvantages, so I still believe there is merit in maintaining both modes. As a result, I've modified it to incorporate the modes.

If you wish to propose a new scaling mode, you could contribute by adding it as an additional mode.

— Reply to this email directly, view it on GitHub https://github.com/ltdrdata/ComfyUI-Impact-Pack/issues/528#issuecomment-2011317037, or unsubscribe https://github.com/notifications/unsubscribe-auth/BEU3H3CPWDFIYLZIFYQYQO3YZJ56RAVCNFSM6AAAAABFAVM2KCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJRGMYTOMBTG4 . You are receiving this because you authored the thread.Message ID: @.***>

shanevcantwell commented 7 months ago

OK I have "logarithmic" and "even_pixels" (divides iterations by pixels not sides). I'm just working out a couple display bugs. It probably won't be ready until tonight or tomorrow - I just didn't want you to beat me to these this time. :)

On Thu, Mar 21, 2024 at 7:53 AM Shane Cantwell @.***> wrote:

Nice! I'm pretty close to finishing it up but hitting the 90-90 rule of course. I forked tho and I think correctly. Do I post the whole distro or just changed files?

On Thu, Mar 21, 2024 at 12:38 AM Dr.Lt.Data @.***> wrote:

During the review of this #523 https://github.com/ltdrdata/ComfyUI-Impact-Pack/pull/523, I've noticed that the existing simple mode indeed produces images closer to the original when compared to the new geometric mode. However, each mode has its own advantages and disadvantages, so I still believe there is merit in maintaining both modes. As a result, I've modified it to incorporate the modes.

If you wish to propose a new scaling mode, you could contribute by adding it as an additional mode.

— Reply to this email directly, view it on GitHub https://github.com/ltdrdata/ComfyUI-Impact-Pack/issues/528#issuecomment-2011317037, or unsubscribe https://github.com/notifications/unsubscribe-auth/BEU3H3CPWDFIYLZIFYQYQO3YZJ56RAVCNFSM6AAAAABFAVM2KCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJRGMYTOMBTG4 . You are receiving this because you authored the thread.Message ID: @.***>

ltdrdata commented 7 months ago

Nice! I'm pretty close to finishing it up but hitting the 90-90 rule of course. I forked tho and I think correctly. Do I post the whole distro or just changed files? … On Thu, Mar 21, 2024 at 12:38 AM Dr.Lt.Data @.> wrote: During the review of this #523 <#523>, I've noticed that the existing simple mode indeed produces images closer to the original when compared to the new geometric mode. However, each mode has its own advantages and disadvantages, so I still believe there is merit in maintaining both modes. As a result, I've modified it to incorporate the modes. If you wish to propose a new scaling mode, you could contribute by adding it as an additional mode. — Reply to this email directly, view it on GitHub <#528 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/BEU3H3CPWDFIYLZIFYQYQO3YZJ56RAVCNFSM6AAAAABFAVM2KCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJRGMYTOMBTG4 . You are receiving this because you authored the thread.Message ID: @.>

Just push commit to your repo. And then you can see pull request button in your repo.