Page MenuHome

Screen edge and off-by-one fixes.
ClosedPublic

Authored by Anthony Edlin (krash) on Nov 25 2013, 6:12 AM.

Details

Summary

Old patch redone for current git master.
I think I've got all changes needed since it last broke from trunk.
Details at old patch task: T20968

Here is the short summary of changes.

  1. Fixes an off-by-one error in screen_test_scale() which causes the areas and regions to draw one pixel bigger on the right and top side of the window, therefor hiding one line of pixels.
  2. Fixes an off-by-one error in rct_fits() which causes regions to incorrectly hide even though it would fit inside the area.
  3. Correctly set the limits for the screen edge move operator so it will always go up to AREAMINX and headery.
  4. Change screen_find_active_scredge() so it doesn't show the arrows cursor on the screen edges along the window border.

If I have time, I may go over and in-line comment some places so you can understand order of changes. I have a patch series on my branch but here it's one diff.

Thanks.

Diff Detail

Event Timeline

Ok some more info.

I find it really helps to change the region border colors in region_draw_emboss to visualize this patch. It's the first function in area.c.

It's also important to understand how integer rects are used in blender ui code. Integer rects in the ui code are constructed as a lower left and top right point and are INCLUSIVE. Meaning that if you have a rect's xmin = 10 and xmax = 30 then the total number of pixels is 21. So to get the size of a rect you have to do xmax - xmin + 1. In order to compare sizes of different rcti you can't forget to use the + 1 in conversion from inclusive rect values to a single size value. If you go back from size to inclusive rect offset you have to - 1. This is confusing because the helper functions BLI_rcti_size_x and BLI_rcti_size_y do not do the + 1 for you even though they have size in the name. You can see examples all over blender ui code of having to do + 1 to get correct size value. In order to understand changes you have to understand this.

Finally, it's important to understand that screen verts on the edge of the screen shouldn't be drawn or considered when making the total rect for the areas. You can see this in area_calc_totrct(). But there are several other places in blender where this should also be considered, such as the area move operator and the screen edge cursor setting code. The remaining of the patch is making sure this code works correctly under these circumstances.

I tried to keep the patch only changing things that are already in the code and as small as possible and still be correct. Sometimes looking at these things I want to just rip stuff out, like screen verts/edges, and replace it with a better system, but that's not for this patch.

I'll go over some of the code in-line to help show order of changes.

Hope this helped.

source/blender/editors/screen/area.c
865

Again an off-by-one where we need to compare a size value to another size value. This would make regions hide even if they would fit inside the area rect.

source/blender/editors/screen/screen_edit.c
268

Not really happy with having to pass in winsize on these functions just to check screen edge but it's the convention in other blender functions.

686

These lines are first changes to be made. Clearly we want size because we are comparing it to winsize below so we have to do + 1, like everywhere else in blender's ui code that uses integers. After you do this you should see that borders on regions against the top and right side of window should now show up correctly.

725

Have to adjust size of headery based on if screen verts are at edge because screen verts at edge aren't used to calc area totrct.

source/blender/editors/screen/screen_ops.c
1118

Only draw if we actually change the size of areas, otherwise it would send redraw every mouse move even though we snap to AREAGRID.

This is really great work, I tried quite hard to find mistakes patch but it all seems to check out. The whole screen vertex / edge system is indeed quite complex and could be refactored once.

I'll commit this patch, thanks!