-
Notifications
You must be signed in to change notification settings - Fork 38
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
Somewhat complete documentation? #15
Comments
As an example: The oddest thing is, the same node with the same I detect it with |
I understand that the documentation aspect has been weakened due to a focus on development. It's true that documentation is lacking. There are plans to improve it in the future. And the scenario you described seems like a bug. If I can reproduce it on my end, I'll let you know after the bug has been fixed. |
Thanks for the response 🙏🏻 The image is upscaled multiple times, from 768x448. The resolution at which the issue appeared is 3072x1762 Here's the entire workspace for you to reproduce (it uses a male-focused model, though): The graph might look terrifying at first, but it's quite simple: each horizontal row is cleanup at a specific resolution. All the config is on the left. |
Thank you I will check it. |
Can you share the upscaled image? |
I've changed the graph quite significantly since then. Give me a few minutes, I'll load the workspace I've uploaded here... |
Here it is: However, it seems I've changed some parameters since the last time I made the screenshot. The seam is still there, bit it's moved (it's not across bicep now, but instead along forearm, therefore it's less visible, but it's still there): I'll check my browser history to see if I can find that one, with the clearly visible issue. |
Yeah. Found it and re-generated it just now: |
I cannot reproduce this. NOTE: SimpleDetector is patched. So you need to update Impact Pack if you try this workflow. |
Hmmm... I've updated everything yesterday evening. No node updates since then, only a few commints on the main ComfyUI repo. Let me isolate the issue as much as possible... I'll try to upload an image just before the problematic node. |
Sooo... Just in case, I've updated ComfyUI, too. Here's the isolated part of the graph. Can you reproduce the issue at least with it?
UPD: UPD#2:
|
I'll try reproduce after get home. |
I was able to successfully reproduce the error situation. |
Yaay! Good to know this elusive bug is narrowed down. 🤝 The lack of documentation is still there, though. So, I guess, this issue should stay open until it's resolved (maybe other people would contribute). As for cropping bug, if you'll create a separate issue to keep track of it, please mention me - so I'll subscribe to it, too. |
oh... my mistake... It's not the facedetailer problem. It's crop_factor problem. The phenomenon occurs when crop_factor is 2.0 |
Upon closer inspection, I realized that this wasn't a bug. I misunderstood because this approach isn't commonly used. The reason for combining BBox Detector and SAM is typically to extract only the silhouette of the face. BBox Detector captures the bounding box area of the face, whereas SAM captures the entire person. So, by obtaining the intersection of these two, you can achieve a detailed facial silhouette. The issue is that when capturing areas beyond the face, unlike the usual case where the neck might be cut off, it looks odd when parts of the body are cropped out abruptly. To address this phenomenon, you need to significantly increase the crop_factor of the bbox, altering the region that gets cropped out by SAM. Alternatively, you can adjust the parameters of CLIPSeg to encompass the entire region of interest, ensuring that the detection scope covers that specific area comprehensively. |
2 specifically? Or anything below 2? Or above? For hands fix, I have 3, 4 and even 6 - to give the model a bigger chunk of image for context about body size / proportions.
Since I wasn't able to find it in the docs, my current understanding of what the entire node pack does is very vague, I've just followed examples shown in the tutorials. Currently, I don't even understand why do I need SAM model (or do I?) and how bbox_detector is different from segm_detector, when should I use one and/or the other. If I get it right, with If I get it right again, the whole ...Detailer setup works by:
But if so, why blur is there twice then? On
I can, but... since it's unclear how exactly And for future, this raises up the issue of missing docs. |
Initially, the approach involved working separately on detection functionality through the detector, mask improvement through mask manipulation, and detailing functionality. However, this was later streamlined into a single node called "FaceDetailer," simplifying the frequently used pattern. While FaceDetailer is convenient to use, it can obstruct understanding of its internal workings. It appears that guiding beginners to use FaceDetailer as a preset by hiding the existing approach might have been a mistake on my part, as it might seem overly complex for newcomers. Looking at this tutorial should make it easier to understand each parameter: There are various nodes related to SEGS, so I recommend using them to directly inspect the output from the detector. This will help you gain a better understanding. The reason for applying blur in CLIPSeg is to ensure that adjacent fragmented masks become connected during the blurring process, treating them as a single mask entity. Once they form a cohesive unit, MaskToSEGS recognizes these masks as a singular segment and generates the corresponding SEG. Additionally, the criterion for the crop area is based on the individual masks recognized as cohesive units. The blurring of the mask itself in CLIPSeg isn't crucial since the mask will be binary eventually. What matters is making sure the mask is recognized as a single cohesive entity. |
I've looked at it here (didn't downloaded the workflow itself, though). Seems like I need to start from scratch, learning the theory of what each node does first.
Yeah, I got that. Unfortunately, Anyway, thanks for your guidance. The only thing clear to me now is that nothing is clear (not even things that I thought I had a grasp of) and that I need to go back to step 1. |
Currently, a significant part of nodes/parameters aren't documented at all. E.g., in the main
FaceDetailer
node it's unclear what a half of all the parameters does (everything afterfeather
). Similarly, it's never explained how oneDetailer
orDetector
compares to the other (SAMDetector vs Simple Detector - which is the preferred one by default and when should we use the other?).With the current state of docs, it's basically impossible to learn what exactly a specific node/parameter does and why the node might do something unexpected.
The text was updated successfully, but these errors were encountered: