clipping to zero width/height

crteensy · June 24, 2015

In src/gdisp/gdisp.c:

#if GDISP_NEED_CLIP || GDISP_NEED_VALIDATION
	void gdispGSetClip(GDisplay *g, coord_t x, coord_t y, coord_t cx, coord_t cy) {
		MUTEX_ENTER(g);

		// Best is using hardware clipping
		#if GDISP_HARDWARE_CLIP
			#if GDISP_HARDWARE_CLIP == HARDWARE_AUTODETECT
				if (gvmt(g)->setclip)
			#endif
			{
				g->p.x = x;
				g->p.y = y;
				g->p.cx = cx;
				g->p.cy = cy;
				gdisp_lld_set_clip(g);
			}
			#if GDISP_HARDWARE_CLIP == HARDWARE_AUTODETECT
				else
			#endif
		#endif

		// Worst is using software clipping
		#if GDISP_HARDWARE_CLIP != TRUE
			{
				if (x < 0) { cx += x; x = 0; }
				if (y < 0) { cy += y; y = 0; }
				if (cx <= 0 || cy <= 0 || x >= g->g.Width || y >= g->g.Height) { MUTEX_EXIT(g); return; }
				g->clipx0 = x;
				g->clipy0 = y;
				g->clipx1 = x+cx;	if (g->clipx1 > g->g.Width) g->clipx1 = g->g.Width;
				g->clipy1 = y+cy;	if (g->clipy1 > g->g.Height) g->clipy1 = g->g.Height;
			}
		#endif
		MUTEX_EXIT(g);
	}
#endif

As I understand it, the software clipping does not clip at all when cx or cy is zero. Why? Any why does hardware clipping not check for zero width/height? I'm in a situation where the intersection of two rectangles is the clipping region, which may be empty.

Regards

Christoph

inmarket · June 25, 2015

Having a clip size of zero should clip all display from happening as you suggest.

Currently however setting a zero sized clip region is considered as invalid parameter values and is therefore ignored.

If you believe that shouldn't be the case we will certainly listen however each drawing operation will need to be boundary tested to ensure that it doesn't cause any of the drawing operations to crash.

If you are happy to test them all or even to visually verify that a zero sized clip area will be handled correctly then we will be happy to change the code to suit.

Unfortunately it is the end of the financial year in our country and so I am too busy to do that testing myself currently at least for the next couple of weeks.

Joel Bodenmann · June 25, 2015

Unfortunately it is the end of the financial year in our country and so I am too busy to do that testing myself currently at least for the next couple of weeks.

And I am currently in the middle of my final exams. I will have vacation from Wednesday next week on but the plans are that i will focus 100% of my time on the uGFX-Studio so we can get the beta release out ASAP. However, I will definitively have more time if you need any help.

~ Tectu

crteensy · June 25, 2015

So I'll put together some code for testing and post it here. I'm sure I'll miss some of the relevant test cases though, so please try to have a look.

crteensy · June 25, 2015

So here's my gdispGSetClip():

#if GDISP_NEED_CLIP || GDISP_NEED_VALIDATION
	void gdispGSetClip(GDisplay *g, coord_t x, coord_t y, coord_t cx, coord_t cy) {
		MUTEX_ENTER(g);

		// Best is using hardware clipping
		#if GDISP_HARDWARE_CLIP
			#if GDISP_HARDWARE_CLIP == HARDWARE_AUTODETECT
				if (gvmt(g)->setclip)
			#endif
			{
				g->p.x = x;
				g->p.y = y;
				g->p.cx = cx;
				g->p.cy = cy;
				gdisp_lld_set_clip(g);
			}
			#if GDISP_HARDWARE_CLIP == HARDWARE_AUTODETECT
				else
			#endif
		#endif

		// Worst is using software clipping
		#if GDISP_HARDWARE_CLIP != TRUE
			{
				if (x < 0) { cx += x; x = 0; }
				if (y < 0) { cy += y; y = 0; }
				/**  MODIFIED BY CHRISTOPH **/
//				if (cx <= 0 || cy <= 0 || x >= g->g.Width || y >= g->g.Height) { MUTEX_EXIT(g); return; }
				if (cx < 0 || cy < 0 || x >= g->g.Width || y >= g->g.Height) { MUTEX_EXIT(g); return; }
				g->clipx0 = x;
				g->clipy0 = y;
				g->clipx1 = x+cx;	if (g->clipx1 > g->g.Width) g->clipx1 = g->g.Width;
				g->clipy1 = y+cy;	if (g->clipy1 > g->g.Height) g->clipy1 = g->g.Height;
			}
		#endif
		MUTEX_EXIT(g);
	}
#endif

And my test code:

#include "gfx.h"
void draw()
{
  static font_t font = gdispOpenFont("UI2 Narrow");
  if(font == NULL)
  {
    printf("could not open font\n");
    while(1);
  }
  coord_t width = gdispGGetWidth(GDISP);
  coord_t height = gdispGGetHeight(GDISP);
  gdispGDrawPixel(GDISP, -10, -10, Black); // off screen
  gdispGDrawPixel(GDISP, 10, 10, Black); // on screen
  gdispGDrawPixel(GDISP, width, height, Black); // off screen

  // lines, on screen
  coord_t x0 = 20, y0 = 20;
  gdispGDrawLine(GDISP, x0, y0, x0+14, y0-0, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+10, y0-10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+0, y0-14, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-10, y0-10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-14, y0-0, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-10, y0+10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-0, y0+14, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+10, y0+10, Black);

  // lines, off screen
  x0 = -20; y0 = -20;
  gdispGDrawLine(GDISP, x0, y0, x0+14, y0-0, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+10, y0-10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+0, y0-14, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-10, y0-10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-14, y0-0, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-10, y0+10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-0, y0+14, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+10, y0+10, Black);

  // lines, off screen
  x0 = width+20; y0 = height+20;
  gdispGDrawLine(GDISP, x0, y0, x0+14, y0-0, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+10, y0-10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+0, y0-14, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-10, y0-10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-14, y0-0, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-10, y0+10, Black);
  gdispGDrawLine(GDISP, x0, y0, x0-0, y0+14, Black);
  gdispGDrawLine(GDISP, x0, y0, x0+10, y0+10, Black);

  // circle, on screen
  x0 = 50; y0 = 20;
  gdispGDrawCircle(GDISP, x0, y0, 14, Black);
  // circle, off screen
  x0 = -50; y0 = -20;
  gdispGDrawCircle(GDISP, x0, y0, 14, Black);
  // circle, off screen
  x0 = width+50; y0 = height+20;
  gdispGDrawCircle(GDISP, x0, y0, 14, Black);

  // string, on screen
  x0 = 80; y0 = 20;
  gdispGDrawString(GDISP, x0, y0, "asdf", font, Black);
  // string, off screen
  x0 = -80; y0 = -20;
  gdispGDrawString(GDISP, x0, y0, "asdf", font, Black);
  // string, off screen
  x0 = width+80; y0 = height+20;
  gdispGDrawString(GDISP, x0, y0, "asdf", font, Black);

  // filled string, on screen
  x0 = 120; y0 = 20;
  gdispGFillString(GDISP, x0, y0, "asdf", font, Black, Green);
  // filled string, off screen
  x0 = -120; y0 = -20;
  gdispGFillString(GDISP, x0, y0, "asdf", font, Black, Green);
  // filled string, off screen
  x0 = width+120; y0 = height+20;
  gdispGFillString(GDISP, x0, y0, "asdf", font, Black, Green);

}

int main(int argc, char** argv)
{
  gfxInit();
  gdispClear(White);
  while(1)
  {
    gdispGUnsetClip(GDISP);
    draw();
    gdispGSetClip(GDISP, 50, 50, 0, 0);
    draw();

    printf("not crashed yet\n");
    gfxSleepMilliseconds(100);
  }
}

and the output is

not crashed yet
not crashed yet
not crashed yet
not crashed yet
not crashed yet
(... and so on ...)

Basically everything that uses drawpixel_clip() seems to work fine, as well as the string drawing function I've tested. This is really just a tiny bit of coverage, but it's promising given that so many functions just use drawpixel_clip() to draw pixels.

Regards

Christoph

inmarket · July 8, 2015

I have finally had time to do a visual peruse of the drawing code to make sure that all the clipping would work for a zero sized area.

Subsequently I have changed the behaviour of gdispSetClip() to allow zero size clipping regions as you suggested.

The forum note is here: http://forum.ugfx.org/viewtopic.php?f=9&t=245

Thanks for the note and your work on researching this.

Sign In

clipping to zero width/height

Recommended Posts

crteensy

inmarket

Joel Bodenmann

crteensy

crteensy

inmarket

Create an account or sign in to comment

Create an account

Sign in

Documentation

Forums