Part of T49043
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
2 questions:
- Several glColors have been left in (e.g. lines 233-239) because they are used to color draw_circle. draw_circle depends on gluDisk. Do you want to rewrite glu dependent parts using pure gawain?
1b) I am also not sure how to apply gawain to ED_mask_draw_region to get rid of that one glColor (828)
- mask_draw_curve_type uses a mix of immediate and VAO, and I am not sure how to rewrite this using gawain.
Thanks!
Comment Actions
- draw_circle -- or anything that uses GLU -- should be replaced. We already have imm_draw_lined_circle & imm_draw_filled_circle helper functions.
1a) We can revisit ED_mask_region_draw later, once we figure out how color is used when drawing the image.
- For mask_curve_draw_type, what values do you see for tot_point ? I would imagine small but I haven't tested. glEnableClientState is not part of core profile so this code does need to change.
| source/blender/editors/mask/mask_draw.c | ||
|---|---|---|
| 205 | Must use immUniformColor here, to match the UNIFORM_COLOR shader. The 3ub or 3ubv version is good since alpha = 0xff. | |
Comment Actions
- Implemented draw circle via immediate helper functions. Is there a way to pass a uniform for scaling to the shader for the x/yaxis specific scaling? Right now I am ignoring y axis scaling and it is so small I can't really tell the difference. Or should there be a imm_draw_circle_axis_scaled?
1a) Haven't looked at the color usage yet
- tot_point seems to be the number of all the points in the spline, so it's not small. I have a later diff built on this where I'm writing mask_curve_draw_type using a VBO, however I cannot bindAttribLocation using the builtin shaders API/builtin basic shader API. When we figure out 1/1a, should I show you my work, or is it outside the scope of immediate mode replacement using gawain?
Comment Actions
Converting to use a VBO is outside the call for help, but definitely fits into the larger project. What do you have in mind?
Comment Actions
Here's my WIP rewrite of mask_curve_draw_type using modern OpenGL, but using the builtin shader API. I think it will work, but the only thing stopping me is the commented part on line 459. I didn't see an interface to the basic shader API that allows binding an attribute array. Is this something that needs to be added or did I miss something? What are your thoughts?
Comment Actions
Hey @Ryan Reyes (ryry), sorry for taking forever to get back to this. Here are some more notes!
| source/blender/editors/mask/mask_draw.c | ||
|---|---|---|
| 444 | It's ok to name these spline_VAO, spline_VBO. Quicker to read at a glance. | |
| 454 | Each point is 2 * float, so total size should be tot_point * 2 * sizeof(GL_FLOAT) | |
| 459 | For now we only "get" attribute locations for builtin shaders, never "bind" them. Should be good without this line. | |
| 470–471 | Could use immUniformColor4ub & get rid of color_loc, color_scale. Here and in other nearby places. | |
| 508 | Delete this & the matching glEnableClientState; they're part of the old pre-VBO API. | |
| 552 | Since the whole VAO will be deleted, I think this can be removed. | |
| 899–904 | This function only draws if masklay != NULL. One line (2 vertices) per MaskLayerShape. Put all this setup code inside the if (masklay) block & count the number of lines needed. | |
Comment Actions
I'm not sure if separate x/y scaling will make any difference here. Hopefully not, so we can use simpler methods! We can look closer & decide after the rest of the code is working.
Comment Actions
Happy New Year! Thanks for finally getting back to me.
Regarding ED_mask_draw_region, it seems that ED_mask_draw_region uses glColor to color a bunch of pixels drawn with the old API, using push and popMatrix.
Thanks a bunch for your help and feedback!
| source/blender/editors/mask/mask_draw.c | ||
|---|---|---|
| 459 | Oh okay! Right now, though, it still can't draw the spline to the screen, and I thought that being unable to bind the array was preventing that. Am I missing something else? | |
| 899–904 | Is it better to iterate over the whole linked list to get the number of lines, or could I put the immBegin/End within the for loop? | |
| source/blender/editors/mask/mask_draw.c | ||
|---|---|---|
| 899–904 | int num_lines = BLI_listbase_count(masklay->splines_shapes); It does the same thing as your loop, but keeps things readable here. | |
Comment Actions
Committed to blender2.8 with some modifications:
- used imm mode instead of creating VAO & VBO (vertex count is rather small)
- took your x/y scale observation & ran with it
Left some TODOs for later!