adafruit / TFTLCD-Library

Arduino library for 8-bit TFT LCDs such as ILI9325, ILI9328, etc
http://www.ladyada.net/products/tfttouchbreakout/ and http://www.ladyada.net/products/tfttouchshield
319 stars 260 forks source link

Bug in fillTriangle() #2

Closed owendelong closed 11 years ago

owendelong commented 12 years ago

This code demonstrates the problem:

You can also refer to my more complete description of the problem in the adafruit forums where it seems the solution offered by adafruit to any request is merely to ask the poster to go do something else.

http://forums.adafruit.com/viewtopic.php?f=47&t=26204


#include <TFTLCD.h>

// Modify as needed for your configuration
#define LCD_RESET A4
#define LCD_CS A3
#define LCD_CD A2
#define LCD_WR A1
#define LCD_RD A0
#define LCD_BL 8

#define FACECENTRX 63
#define FACECENTRY 63

TFTLCD lcd(LCD_CS, LCD_CD, LCD_WR, LCD_RD, LCD_RESET);

// This should initialize the display and draw a diamond shaped hand rotating around the dial ala a second hand on a clock.
// This example is reduced from an actual clock application to the minimum necessary to replicate the library bug
void setup()
{
  lcd.reset();
  lcd.initDisplay();
  lcd.fillScreen(lcd.Color565(0,0,0));
  lcd.setRotation(2);
  pinMode(LCD_BL, OUTPUT);
  digitalWrite(LCD_BL, HIGH);
  int i;
  while (1)
  {
    for(i=0; i<360; i+=6)
    {
      drawHand(lcd, i, 46, 255,255,255, 10);
      delay(1000);
      drawHand(lcd, i, 46, 0, 0, 0, 10);
    }
  }
}

void drawHand(TFTLCD D2, int a, int d, byte r, byte g, byte b, int width)
{
  float c,w, z;
  int dh_x2, dh_y2;
  int dh_x1, dh_y1;
  unsigned int co;
  float rad;
  co=D2.Color565(r,g,b);
  c = ((a+270)%360); // orient correctly
  z = c = c * PI/180;
  // Draw hands as polygons (fast)
  dh_x1 = cos(c)*d+FACECENTRX; dh_y1 = sin(c)*d+FACECENTRY; // tip of hand
  w=width*PI/180;
  c -= w/2;  // Widen (go counterclockwise w/2 radians (== width/2 degrees)
  rad = d*(d<40 ? 0.6 : 0.8);
  dh_x2 = cos(c)*rad+FACECENTRX; dh_y2 = sin(c)*rad+FACECENTRY; // Counterclockwise wide point
  D2.fillTriangle(FACECENTRX, FACECENTRY, dh_x1, dh_y1, dh_x2, dh_y2, D2.Color565(r, g, b)); // Draw counterclockwise half of hand
  c += w;    // Widen other direction (clockwise w radians == width degrees from counterclockwise edge)
  dh_x2 = cos(c)*rad+FACECENTRX; dh_y2 = sin(c)*rad+FACECENTRY; // Colckwise wide point
  D2.fillTriangle(FACECENTRX, FACECENTRY, dh_x1, dh_y1, dh_x2, dh_y2, D2.Color565(r, g, b)); // Draw clockwise half of hand
}

void loop()
{
}
owendelong commented 12 years ago

I've narrowed this down a little bit more.

The problem seems to occur when (after sorting the coordinate pairs such that y0<=y1<=y2, you are left with a case where x0<x2<x1.

It does not seem to matter whether or not any of the x values are equal or whether or not any of the y values are equal.

I'm continuing to work on figuring out a fix.

owendelong commented 12 years ago

Resolved...

Here's the deal... Sometimes the algorithm passes to drawHorizontalLine a value where x1>x2. This results in a line width being passed to drawHorizontalLine which is a negative signed long int. drawHorizontalLine expects an unsigned 16 bit integer.

Solution: reorder the arguments to drawHorizontalLine and reverse the computation for the width of the line when necessary.

Essentially replace occurrences of:


      drawHorizontalLine(sx1/1000, sy, (sx2-sx1)/1000, color);

With:


      if (sx2 < sx1)
      {
        drawHorizontalLine(sx2/1000, sy, (sx1-sx2)/1000, color);
      }
      else
      {
        drawHorizontalLine(sx1/1000, sy, (sx2-sx1)/1000, color);
      }

Hopefully when you guys merge this into the library, you can also merge in my improvements to drawChar.

owendelong commented 12 years ago

There are still occasionally odd results with some triangles. It's working well enough in my application that I probably won't have time to track it down, but at least the above code changes result in consistent results and no screen-filling errors.

PaintYourDragon commented 11 years ago

Resolved in one of the interim commits since this was originally posted.