Part of T49043, to replace gl immediate calls with Gawain
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- imm.view2d.c.UI_view2d_multi_grid_draw
- Build Status
Buildable 301 Build 301: arc lint + arc unit
Event Timeline
Comment Actions
Replicating loops & logic is a good first way to get the primitive counts. But it will become a nesting area for future bugs. You can calculate an upper bound then use immBeginAtMost. Keep hacking away! I spent at least 1.5 full days on the 3D view's ortho grids by the way.
Comment Actions
Having two very similar for loops in this function doesn't seem that elegant. Can anyone recommend any alternatives?
In diff 7834 I changed this to make an estimate as per merwin's request.
| source/blender/editors/interface/view2d.c | ||
|---|---|---|
| 1526 | There is a double negative here. I tested with removing it (as well as the other ones) and it didn't look like it messed with anything. Should I go ahead and submit a diff with those removed (also at lines 1486, 1521, and 1532)? | |
Comment Actions
I tested earlier and it was working but now issues with double assign of attributes.
I'll do more testing after I wrap my head around these loops.
| source/blender/editors/interface/view2d.c | ||
|---|---|---|
| 1528 | Using immSkipAttrib means you can't setup color ahead of time here, have to remove this line and line 1520 to stop asserts. | |
Comment Actions
I have to say, there is a lot wrong with this function. I think this is a case where just converting to gawain isn't really all that's needed because the original code isn't very good. Really have to try to understand what is going on and it's taken me a bit. Added a first pass review. There is more but have to go to work now so I'll do more tonight.
| source/blender/editors/interface/view2d.c | ||
|---|---|---|
| 1523 | Few things here. I think you need to cast the ((v2d->cur.xmax) / lstep) to integers. I'm getting assert below at immBeginAtMost with vertex_count of 149. Other thing is that you can't just use "cur.xmax". If you put axis close to right of screen then xmax is close to 0, which will give you almost no vertexes. Try it and it'll assert because you go past vertex count. You probably want difference of xmax and xmin. Using totlevels is ok but it gives you way more verts then needed because verts needed for other levels is reduced. Actually it skips verts on first levels if they are on next level so you may not even need to multiply by totlevels, you'll have to test. | |
| 1563–1564 | Not sure why drawing X and Y axis is inside the levels loop. Doesn't this just draw them repeatedly for no reason? It was inside the loop in the old code too, strange. Maybe I miss something. | |
Comment Actions
- only draw axis once now
- Removed the double negative
- Fixed vertex counting
- small simplification
Comment Actions
This looks much cleaner than the first revision. After @Anthony Edlin (krash) accepts I'll put it in 2.8.
| source/blender/editors/interface/view2d.c | ||
|---|---|---|
| 1526–1527 | C99 style thing -- can declare these below, first time they're used: int i = (int)(v2d ... float start = i * lstep; | |
| 1526–1527 | for (int level = 0 ... Delete declaration of level earlier this function. | |
| 1574 | Empty (code) line between these 2 (GL) lines, for clarity. | |
Comment Actions
- Merge branch 'blender2.8' into imm.view2d.c.UI_view2d_multi_grid_draw
- small code cleanup
Comment Actions
I was still getting asserts in some cases, think I've found all corner cases remaining now.
| source/blender/editors/interface/view2d.c | ||
|---|---|---|
| 1515–1524 | Can remove these, cur_xmin_positive and cur_ymin_positive | |
| 1522 | You need a +1 on this for cases where cur spans just over grid lines. Simple example, let's say lstep is 40 (what it is on my setup), xmin is 30 and xmax is 90. xmax-xmin is 60. 60/40 turns into 1. But grid lines are drawn at 40 and 80 because xmin starts at 30. | |
| 1535 | Need to increment "i" here if cur.xmin is positive before using it to set "start". Simple example again, converting a float to int throws away the decimal, so 1.5 becomes 1, and -1.5 becomes -1. This shows that converting to int will always round towards 0. This is fine for negative cur because it rounds up, but if cur is positive it will round down and you will be drawing the first grid line outside the cur rect. | |
| 1552 | Same here, need to increment "i" if ymin > 0 | |
Comment Actions
- Merge branch 'blender2.8' into imm.view2d.c.UI_view2d_multi_grid_draw
- removed unused variables
- added for a case where "cur spans just over grid lines"
- Fixing a case where to need to incmrent "i"