Hi @Emiel Bos (EmielBoss), thanks for making a patch.

I looked into this a little bit. There are some things that are still confusing to me. If you select 2 edges on opposite sides of a closed edge loop and if the number of edges is not even (such as a circle with 9 edges), then it will still do a grid fill but just add a triangle where needed. Which makes me wonder, does it really need an even number of edges in a circle to work? Maybe just try removing the even number of edges check and it might work, although it will add a triangle where needed. Maybe in the past the bmesh grid fill operator needed even edges but it doesn't seem to any more. I'd give it a try and thorough test and maybe you won't need to change the error message.

Still if we only want to allow even numbered closed border edge loops to work, I think this is not the correct place to raise the error. It should probably be raised when the odd numbering is detected, earlier in the process. Probably in edbm_fill_grid_exec() or edbm_fill_grid_prepare()? You can use BKE_report() to do stuff like that.

The other thing I wonder about is terminology, throughout the code "edge loop" is used as a set of connected edges, but it can be either open or closed. "Closed" meaning that the first and last edges in the loop are connected. Also there is the term "Border" edge loop which indicates 1 side doesn't have any faces connected to it. What it seems the grid fill operator really wants is 2 open edge loops selected on a single closed border edge loop. I'm not sure of the correct terminology to use, but "Select an even number of edges in a single edge loop" confused me. I could select 4 seperate and unconnected edges on a single border closed edge loop and it would seem to fit that description but I'd still get the error.

Hey @Anthony Edlin (krash), thank you for the feedback and for looking at my patch! :)

The other thing I wonder about is terminology, throughout the code "edge loop" is used as a set of connected edges, but it can be either open or closed. "Closed" meaning that the first and last edges in the loop are connected. Also there is the term "Border" edge loop which indicates 1 side doesn't have any faces connected to it.

Thanks for clearing this up!

What it seems the grid fill operator really wants is 2 open edge loops selected on a single closed border edge loop. I'm not sure of the correct terminology to use, but "Select an even number of edges in a single edge loop" confused me. I could select 4 separate and unconnected edges on a single border closed edge loop and it would seem to fit that description but I'd still get the error.

I thought that "edge loops" were by definition closed, because a "loop" by definition has its end connected to its start. However, your definitions makes more sense, because I wouldn't know how else you'd call an "open edge loop". It's also what the glossary dictates.
In my mind, I distinguished two cases for Grid Fill, one where the user selects a subset of a closed edge loop and one where the user selects all edges of the closed edge loop. With "Select an even number of edges in a single edge loop" I meant that the user should select an even number of edges in case he/she had the entire edge loop selected, since I thought the error didn't appear when selecting a subset (and even that isn't true, because the error also appears when selecting one connected subset of the closed edge loop). In short, I messed up. My apologies!

Maybe just try removing the even number of edges check and it might work, although it will add a triangle where needed. Maybe in the past the bmesh grid fill operator needed even edges but it doesn't seem to any more.

Grid Fill-ing on a selection of two or more edge loops on a closed edge loop always works, and selecting only one subset of a closed edge loop never works, so the following will be about selecting the entire closed edge loop. I tried disabling the even-number-of-edges check, but then Grid Fill-ing a closed edge loop with uneven number of edges yields a "Closed loops unsupported". Disabling that as well results in "Connecting edge loops overlap", so this doesn't seem like a good approach. Some options (for one entirely selected closed edge loop) are:

• Just enforce that a closed edge loop needs an even number of edges as before, but in the correct place (as you suggested) and with a clearer error message: "Can't fill one entirely selected closed edge loop with an uneven number of edges. Either select two opposing, disconnected edge loops or use regular Fill." However, this seems like an inconvenient and unnecessary long error message, and isn't very user-friendly.
• Automatically switch to regular Fill, but this creates triangles, whereas Grid Fill seems to prefer quads (or alternatively faces with >4 vertices) as much as possible. Seems very opaque and counter to Grid Fill's aim.
• Try to get the same result we get when Grid Fill-ing two opposing edge loops (on the closed edge loop). This sounds like the best option, but requires me to dive deeper into the code, so this might take a while. Maybe I can create a separate execution/control flow with a single_closed_edge_loop boolean, or modify the selection and try to make it two opposing edge loops.

And then there's the edge case of a closed edge loop with three edges. Right now, Grid Fill can't fill such triangles at all. Is it an idea to catch this case and fill it using regular Fill? Or better leave that be?

I thought that "edge loops" were by definition closed, because a "loop" by definition has its end connected to its start.

Yeah I think as far as the interface and users are concerned it's probably ok to just use the term "edge loop" somewhat ambiguously. It's just that when talking about the code and operators you need more specific terms, so I just defined these randomly so we could talk about it.

I tried disabling the even-number-of-edges check, but then Grid Fill-ing a closed edge loop with uneven number of edges yields a "Closed loops unsupported". Disabling that as well results in "Connecting edge loops overlap", so this doesn't seem like a good approach.

It seems like you are removing the checks in bmo_grid_fill_exec() to do this and I think that is the wrong spot. The bmesh operator I think always expects 2 open edge loops on a single border closed edge loop and should probably stay that way. But in the case of when you have a single border closed edge loop selected, it must split that edge loop into 2 open edge loops before the bmesh operator is run, probably in the operator functions edbm_fill_grid_exec() or edbm_fill_grid_prepare(). It must only split the closed edge loop if it has an even number of edges, but if you remove the check for an even number of edges it may split it anyways. It seems the bmesh operator can handle 2 open edge loops with different numbers of edges.

I think to keep it simple for now and to just fix this error message we should either, make it so the code splits a single border closed edge loop with uneven number of edges, or make the operator cancel and report an error saying that you must ensure that your single border edge loop has an even number of edges. Clearly grid fill could have more complicated features but that can be done separately.

We probably want someone on the Modeling module to check this.