NLA SoC: Cleanup of Keyframe Drawing code for DopeSheet

* Removed the glaMapping stuff in favour of remapping the keyframes manually. The old code was causing some mess, and not really working well for the DopeSheet with various mappings being used.

* Fixed NLA-mapped selection in DopeSheet. Clicking on individual keyframes should now work ok. More testing required though.

* Keyframes in DopeSheet are now drawn fully using OpenGL (instead of icons). They look less tactile now, but this may be compensated with in terms of speed? Previously I disabled this type of drawing due to issues with some cards. Enabled again for now, but mainly because I couldn't get the icons to line up nicely in screenspace...

* Borderselect in DopeSheet. I've had a look at issues with it selecting the wrong channel's keyframes. The issues don't seem to be solved yet though... will look again tomorrow.

2 files changed, 56 insertions, 106 deletions

diff --git a/source/blender/editors/space_action/action_draw.c b/source/blender/editors/space_action/action_draw.c
index 910b9733bc8..743cc1f128e 100644
--- a/source/blender/editors/space_action/action_draw.c
+++ b/source/blender/editors/space_action/action_draw.c
@@ -960,27 +960,8 @@ void draw_channel_names(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 /* ************************************************************************* */
 /* Keyframes */
 
-ActKeysInc *init_aki_data(bAnimContext *ac, bAnimListElem *ale)
-{
-	static ActKeysInc aki;
-	
-	/* no need to set settings if wrong context */
-	if ((ac->data == NULL) || ELEM(ac->datatype, ANIMCONT_ACTION, ANIMCONT_DOPESHEET)==0)
-		return NULL;
-	
-	/* if strip is mapped, store settings */
-	aki.adt= ANIM_nla_mapping_get(ac, ale);
-	
-	if (ac->datatype == ANIMCONT_DOPESHEET)
-		aki.ads= (bDopeSheet *)ac->data;
-	else
-		aki.ads= NULL;
-	aki.actmode= ac->datatype;
-		
-	/* always return pointer... */
-	return &aki;
-}
-
+/* extra padding for lengths (to go under scrollers) */
+#define EXTRA_SCROLL_PAD	100.0f
 
 /* draw keyframes in each channel */
 void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
@@ -990,13 +971,11 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 	int filter;
 	
 	View2D *v2d= &ar->v2d;
+	bDopeSheet *ads= &saction->ads;
 	AnimData *adt= NULL;
-	gla2DDrawInfo *di;
-	rcti scr_rct;
 	
-	int act_start, act_end, dummy;
+	float act_start, act_end, y;
 	int height, items;
-	float y, sta, end;
 	
 	char col1[3], col2[3];
 	char col1a[3], col2a[3];
@@ -1006,6 +985,7 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 	/* get theme colors */
 	UI_GetThemeColor3ubv(TH_BACK, col2);
 	UI_GetThemeColor3ubv(TH_HILITE, col1);
+	
 	UI_GetThemeColor3ubv(TH_GROUP, col2a);
 	UI_GetThemeColor3ubv(TH_GROUP_ACTIVE, col1a);
 	
@@ -1013,26 +993,14 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 	UI_GetThemeColor3ubv(TH_DOPESHEET_CHANNELSUBOB, col2b);
 	
 	/* set view-mapping rect (only used for x-axis), for NLA-scaling mapping with less calculation */
-	scr_rct.xmin= ar->winrct.xmin + ar->v2d.mask.xmin;
-	scr_rct.ymin= ar->winrct.ymin + ar->v2d.mask.ymin;
-	scr_rct.xmax= ar->winrct.xmin + ar->v2d.hor.xmax;
-	scr_rct.ymax= ar->winrct.ymin + ar->v2d.mask.ymax; 
-	di= glaBegin2DDraw(&scr_rct, &v2d->cur);
 
 	/* if in NLA there's a strip active, map the view */
 	if (ac->datatype == ANIMCONT_ACTION) {
 		adt= ANIM_nla_mapping_get(ac, NULL);
 		
-		if (adt)
-			ANIM_nla_mapping_draw(di, adt, 0);
-		
 		/* start and end of action itself */
-		calc_action_range(ac->data, &sta, &end, 0);
-		gla2DDrawTranslatePt(di, sta, 0.0f, &act_start, &dummy);
-		gla2DDrawTranslatePt(di, end, 0.0f, &act_end, &dummy);
-		
-		if (adt)
-			ANIM_nla_mapping_draw(di, adt, 1);
+		// TODO: this has not had scaling applied
+		calc_action_range(ac->data, &act_start, &act_end, 0);
 	}
 	
 	/* build list of channels to draw */
@@ -1063,7 +1031,7 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 		if ( IN_RANGE(yminc, v2d->cur.ymin, v2d->cur.ymax) ||
 			 IN_RANGE(ymaxc, v2d->cur.ymin, v2d->cur.ymax) ) 
 		{
-			int frame1_x, channel_y, sel=0;
+			int sel=0;
 			
 			/* determine if any need to draw channel */
 			if (ale->datatype != ALE_NONE) {
@@ -1102,8 +1070,6 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 				}
 				
 				if (ELEM(ac->datatype, ANIMCONT_ACTION, ANIMCONT_DOPESHEET)) {
-					gla2DDrawTranslatePt(di, v2d->cur.xmin, y, &frame1_x, &channel_y);
-					
 					switch (ale->type) {
 						case ANIMTYPE_SCENE:
 						case ANIMTYPE_OBJECT:
@@ -1139,36 +1105,32 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 					}
 					
 					/* draw region twice: firstly backdrop, then the current range */
-					glRectf((float)frame1_x,  (float)channel_y-ACHANNEL_HEIGHT_HALF,  (float)v2d->hor.xmax,  (float)channel_y+ACHANNEL_HEIGHT_HALF);
+					glRectf(v2d->cur.xmin,  (float)y-ACHANNEL_HEIGHT_HALF,  v2d->cur.xmax+EXTRA_SCROLL_PAD,  (float)y+ACHANNEL_HEIGHT_HALF);
 					
 					if (ac->datatype == ANIMCONT_ACTION)
-						glRectf((float)act_start,  (float)channel_y-ACHANNEL_HEIGHT_HALF,  (float)act_end,  (float)channel_y+ACHANNEL_HEIGHT_HALF);
+						glRectf(act_start,  (float)y-ACHANNEL_HEIGHT_HALF,  act_end,  (float)y+ACHANNEL_HEIGHT_HALF);
 				}
 				else if (ac->datatype == ANIMCONT_SHAPEKEY) {
-					gla2DDrawTranslatePt(di, 1, y, &frame1_x, &channel_y);
-					
 					/* all frames that have a frame number less than one
 					 * get a desaturated orange background
 					 */
 					glColor4ub(col2[0], col2[1], col2[2], 0x22);
-					glRectf(0.0f, (float)channel_y-ACHANNEL_HEIGHT_HALF, (float)frame1_x, (float)channel_y+ACHANNEL_HEIGHT_HALF);
+					glRectf(0.0f, (float)y-ACHANNEL_HEIGHT_HALF, 1.0f, (float)y+ACHANNEL_HEIGHT_HALF);
 					
 					/* frames one and higher get a saturated orange background */
 					glColor4ub(col2[0], col2[1], col2[2], 0x44);
-					glRectf((float)frame1_x, (float)channel_y-ACHANNEL_HEIGHT_HALF, (float)v2d->hor.xmax,  (float)channel_y+ACHANNEL_HEIGHT_HALF);
+					glRectf(1.0f, (float)y-ACHANNEL_HEIGHT_HALF, v2d->cur.xmax+EXTRA_SCROLL_PAD,  (float)y+ACHANNEL_HEIGHT_HALF);
 				}
 				else if (ac->datatype == ANIMCONT_GPENCIL) {
-					gla2DDrawTranslatePt(di, v2d->cur.xmin, y, &frame1_x, &channel_y);
-					
 					/* frames less than one get less saturated background */
 					if (sel) glColor4ub(col1[0], col1[1], col1[2], 0x22);
 					else glColor4ub(col2[0], col2[1], col2[2], 0x22);
-					glRectf(0.0f, (float)channel_y-ACHANNEL_HEIGHT_HALF, (float)frame1_x, (float)channel_y+ACHANNEL_HEIGHT_HALF);
+					glRectf(0.0f, (float)y-ACHANNEL_HEIGHT_HALF, v2d->cur.xmin, (float)y+ACHANNEL_HEIGHT_HALF);
 					
 					/* frames one and higher get a saturated background */
 					if (sel) glColor4ub(col1[0], col1[1], col1[2], 0x44);
 					else glColor4ub(col2[0], col2[1], col2[2], 0x44);
-					glRectf((float)frame1_x, (float)channel_y-ACHANNEL_HEIGHT_HALF, (float)v2d->hor.xmax,  (float)channel_y+ACHANNEL_HEIGHT_HALF);
+					glRectf(v2d->cur.xmin, (float)y-ACHANNEL_HEIGHT_HALF, v2d->cur.xmax+EXTRA_SCROLL_PAD,  (float)y+ACHANNEL_HEIGHT_HALF);
 				}
 			}
 		}
@@ -1195,36 +1157,29 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 		{
 			/* check if anything to show for this channel */
 			if (ale->datatype != ALE_NONE) {
-				ActKeysInc *aki= init_aki_data(ac, ale); 
 				adt= ANIM_nla_mapping_get(ac, ale);
 				
-				if (adt)
-					ANIM_nla_mapping_draw(di, adt, 0);
-				
 				/* draw 'keyframes' for each specific datatype */
 				switch (ale->datatype) {
 					case ALE_SCE:
-						draw_scene_channel(di, aki, ale->key_data, y);
+						draw_scene_channel(v2d, ads, ale->key_data, y);
 						break;
 					case ALE_OB:
-						draw_object_channel(di, aki, ale->key_data, y);
+						draw_object_channel(v2d, ads, ale->key_data, y);
 						break;
 					case ALE_ACT:
-						draw_action_channel(di, aki, ale->key_data, y);
+						draw_action_channel(v2d, adt, ale->key_data, y);
 						break;
 					case ALE_GROUP:
-						draw_agroup_channel(di, aki, ale->data, y);
+						draw_agroup_channel(v2d, adt, ale->data, y);
 						break;
 					case ALE_FCURVE:
-						draw_fcurve_channel(di, aki, ale->key_data, y);
+						draw_fcurve_channel(v2d, adt, ale->key_data, y);
 						break;
 					case ALE_GPFRAME:
-						draw_gpl_channel(di, aki, ale->data, y);
+						draw_gpl_channel(v2d, ads, ale->data, y);
 						break;
 				}
-				
-				if (adt) 
-					ANIM_nla_mapping_draw(di, adt, 1);
 			}
 		}
 		
@@ -1236,16 +1191,11 @@ void draw_channel_strips(bAnimContext *ac, SpaceAction *saction, ARegion *ar)
 
 	/* black line marking 'current frame' for Time-Slide transform mode */
 	if (saction->flag & SACTION_MOVING) {
-		int frame1_x;
-		
-		gla2DDrawTranslatePt(di, saction->timeslide, 0, &frame1_x, &dummy);
-		cpack(0x0);
+		glColor3f(0.0f, 0.0f, 0.0f);
 		
 		glBegin(GL_LINES);
-			glVertex2f((float)frame1_x, (float)v2d->mask.ymin - 100);
-			glVertex2f((float)frame1_x, (float)v2d->mask.ymax);
+			glVertex2f(saction->timeslide, v2d->cur.ymin-EXTRA_SCROLL_PAD)
+			glVertex2f(saction->timeslide, v2d->cur.ymax);
 		glEnd();
 	}
-	
-	glaEnd2DDraw(di);
 }
diff --git a/source/blender/editors/space_action/action_select.c b/source/blender/editors/space_action/action_select.c
index 5b87c04b311..8f95f5ea52d 100644
--- a/source/blender/editors/space_action/action_select.c
+++ b/source/blender/editors/space_action/action_select.c
@@ -223,7 +223,8 @@ static void borderselect_action (bAnimContext *ac, rcti rect, short mode, short
 	BeztEditFunc ok_cb, select_cb;
 	View2D *v2d= &ac->ar->v2d;
 	rctf rectf;
-	float ymin=0, ymax=(float)(-ACHANNEL_HEIGHT);
+	//float ymin=0, ymax=(float)(-ACHANNEL_HEIGHT);
+	float ymin=0, ymax=(float)(-ACHANNEL_HEIGHT_HALF);
 	
 	/* convert mouse coordinates to frame ranges and channel coordinates corrected for view pan/zoom */
 	UI_view2d_region_to_view(v2d, rect.xmin, rect.ymin+2, &rectf.xmin, &rectf.ymin);
@@ -743,12 +744,16 @@ static void mouse_action_keys (bAnimContext *ac, int mval[2], short select_mode,
 	int filter;
 	
 	View2D *v2d= &ac->ar->v2d;
+	bDopeSheet *ads = NULL;
 	int channel_index;
 	short found = 0;
 	float selx = 0.0f;
 	float x, y;
 	rctf rectf;
 	
+	/* get dopesheet info */
+	if (ac->datatype == ANIMCONT_DOPESHEET)
+		ads= ac->data;
 	
 	/* use View2D to determine the index of the channel (i.e a row in the list) where keyframe was */
 	UI_view2d_region_to_view(v2d, mval[0], mval[1], &x, &y);
@@ -773,46 +778,35 @@ static void mouse_action_keys (bAnimContext *ac, int mval[2], short select_mode,
 	else {
 		/* found match - must return here... */
 		AnimData *adt= ANIM_nla_mapping_get(ac, ale);
-		ActKeysInc *aki= init_aki_data(ac, ale);
 		ActKeyColumn *ak;
-		float xmin, xmax;
-		
-		/* apply NLA-scaling correction? */
-		if (adt) {
-			xmin= BKE_nla_tweakedit_remap(adt, rectf.xmin, NLATIME_CONVERT_UNMAP);
-			xmax= BKE_nla_tweakedit_remap(adt, rectf.xmax, NLATIME_CONVERT_UNMAP);
-		}
-		else {
-			xmin= rectf.xmin;
-			xmax= rectf.xmax;
-		}
 		
 		/* make list of keyframes */
+		// TODO: it would be great if we didn't have to apply this to all the keyframes to do this...
 		if (ale->key_data) {
 			switch (ale->datatype) {
 				case ALE_OB:
 				{
 					Object *ob= (Object *)ale->key_data;
-					ob_to_keylist(ob, &anim_keys, NULL, aki);
+					ob_to_keylist(ads, ob, &anim_keys, NULL);
 				}
 					break;
 				case ALE_ACT:
 				{
 					bAction *act= (bAction *)ale->key_data;
-					action_to_keylist(act, &anim_keys, NULL, aki);
+					action_to_keylist(adt, act, &anim_keys, NULL);
 				}
 					break;
 				case ALE_FCURVE:
 				{
 					FCurve *fcu= (FCurve *)ale->key_data;
-					fcurve_to_keylist(fcu, &anim_keys, NULL, aki);
+					fcurve_to_keylist(adt, fcu, &anim_keys, NULL);
 				}
 					break;
 			}
 		}
 		else if (ale->type == ANIMTYPE_GROUP) {
 			bActionGroup *agrp= (bActionGroup *)ale->data;
-			agroup_to_keylist(agrp, &anim_keys, NULL, aki);
+			agroup_to_keylist(adt, agrp, &anim_keys, NULL);
 		}
 		else if (ale->type == ANIMTYPE_GPDATABLOCK) {
 			/* cleanup */
@@ -822,13 +816,17 @@ static void mouse_action_keys (bAnimContext *ac, int mval[2], short select_mode,
 		}
 		else if (ale->type == ANIMTYPE_GPLAYER) {
 			bGPDlayer *gpl= (bGPDlayer *)ale->data;
-			gpl_to_keylist(gpl, &anim_keys, NULL, aki);
+			gpl_to_keylist(ads, gpl, &anim_keys, NULL);
 		}
 		
 		/* loop through keyframes, finding one that was clicked on */
 		for (ak= anim_keys.first; ak; ak= ak->next) {
-			if (IN_RANGE(ak->cfra, xmin, xmax)) {
-				selx= ak->cfra;
+			if (IN_RANGE(ak->cfra, rectf.xmin, rectf.xmax)) {
+				/* set the frame to use, and apply inverse-correction for NLA-mapping 
+				 * so that the frame will get selected by the selection functiosn without
+				 * requiring to map each frame once again...
+				 */
+				selx= BKE_nla_tweakedit_remap(adt, ak->cfra, NLATIME_CONVERT_UNMAP);
 				found= 1;
 				break;
 			}
@@ -884,19 +882,21 @@ static void mouse_action_keys (bAnimContext *ac, int mval[2], short select_mode,
 	}
 	
 	/* only select keyframes if we clicked on a valid channel and hit something */
-	if (ale && found) {
-		/* apply selection to keyframes */
-		if (/*gpl*/0) {
-			/* grease pencil */
-			//select_gpencil_frame(gpl, (int)selx, selectmode);
-		}
-		else if (column) {
-			/* select all keyframes in the same frame as the one we hit on the active channel */
-			actkeys_mselect_column(ac, select_mode, selx);
-		}
-		else {
-			/* select the nominated keyframe on the given frame */
-			actkeys_mselect_single(ac, ale, select_mode, selx);
+	if (ale) {
+		if (found) {
+			/* apply selection to keyframes */
+			if (/*gpl*/0) {
+				/* grease pencil */
+				//select_gpencil_frame(gpl, (int)selx, selectmode);
+			}
+			else if (column) {
+				/* select all keyframes in the same frame as the one we hit on the active channel */
+				actkeys_mselect_column(ac, select_mode, selx);
+			}
+			else {
+				/* select the nominated keyframe on the given frame */
+				actkeys_mselect_single(ac, ale, select_mode, selx);
+			}
 		}
 		
 		/* free this channel */