Fix a compiler issue and optimize drawPixel()

The new compiler in Arduino IDE 1.8.6/1.8.7 has an optimization glitch
where it fails to see the difference between a data structure in RAM
and in PROGMEM when the data is the same and optimizes one out.

drawPixel() is unable to fetch the correct pixel mask from the
bitshift_left array in PROGMEM causing junk to be drawn.

The bitshift_left[] array has now been optimised out and drawPixel()
will function properly. The optimization uses the same number of cycles
and saves 6 bytes.

drawPixel() has also been made static so the Arduboy object pointer is
no longer needlesly passed on to drawPixel() saving more bytes.
This commit is contained in:
Mr.Blinky 2018-11-09 21:10:05 +01:00 committed by Scott Allen
parent 1d49ce5df8
commit 2941ed100f
2 changed files with 27 additions and 46 deletions

View File

@ -322,14 +322,6 @@ void Arduboy2Base::clear()
fillScreen(BLACK); fillScreen(BLACK);
} }
// Used by drawPixel to help with left bitshifting since AVR has no
// multiple bit shift instruction. We can bit shift from a lookup table
// in flash faster than we can calculate the bit shifts on the CPU.
const uint8_t bitshift_left[] PROGMEM = {
_BV(0), _BV(1), _BV(2), _BV(3), _BV(4), _BV(5), _BV(6), _BV(7)
};
void Arduboy2Base::drawPixel(int16_t x, int16_t y, uint8_t color) void Arduboy2Base::drawPixel(int16_t x, int16_t y, uint8_t color)
{ {
#ifdef PIXEL_SAFE_MODE #ifdef PIXEL_SAFE_MODE
@ -342,46 +334,35 @@ void Arduboy2Base::drawPixel(int16_t x, int16_t y, uint8_t color)
uint16_t row_offset; uint16_t row_offset;
uint8_t bit; uint8_t bit;
// uint8_t row = (uint8_t)y / 8;
// row_offset = (row*WIDTH) + (uint8_t)x;
// bit = _BV((uint8_t)y % 8);
// the above math can also be rewritten more simply as;
// row_offset = (y * WIDTH/8) & ~0b01111111 + (uint8_t)x;
// which is what the below assembler does
// local variable for the bitshift_left array pointer,
// which can be declared a read-write operand
const uint8_t* bsl = bitshift_left;
asm volatile asm volatile
( (
"mul %[width_offset], %A[y]\n" // bit = 1 << (y & 7)
"movw %[row_offset], r0\n" "ldi %[bit], 1 \n" //bit = 1;
"andi %A[row_offset], 0x80\n" // row_offset &= (~0b01111111); "sbrc %[y], 1 \n" //if (y & _BV(1)) bit = 4;
"clr __zero_reg__\n" "ldi %[bit], 4 \n"
"add %A[row_offset], %[x]\n" "sbrc %[y], 0 \n" //if (y & _BV(0)) bit = bit << 1;
// mask for only 0-7 "lsl %[bit] \n"
"andi %A[y], 0x07\n" "sbrc %[y], 2 \n" //if (y & _BV(2)) bit = (bit << 4) | (bit >> 4);
// Z += y "swap %[bit] \n"
"add r30, %A[y]\n" //row_offset = y / 8 * WIDTH + x;
"adc r31, __zero_reg__\n" "andi %A[y], 0xf8 \n" //row_offset = (y & 0xF8) * WIDTH / 8
// load correct bitshift from program RAM "mul %[width_offset], %A[y] \n"
"lpm %[bit], Z\n" "movw %[row_offset], r0 \n"
: [row_offset] "=&x" (row_offset), // upper register (ANDI) "clr __zero_reg__ \n"
[bit] "=r" (bit), "add %A[row_offset], %[x] \n" //row_offset += x
[y] "+d" (y), // upper register (ANDI), must be writable #if WIDTH != 128
"+z" (bsl) // is modified to point to the proper shift array element "adc %B[row_offset], __zero_reg__ \n" // only non 128 width can overflow
: [width_offset] "r" ((uint8_t)(WIDTH/8)), #endif
[x] "r" ((uint8_t)x) : [row_offset] "=&x" (row_offset), // upper register (ANDI)
[bit] "=&d" (bit), // upper register (LDI)
[y] "+d" (y) // upper register (ANDI), must be writable
: [width_offset] "r" ((uint8_t)(WIDTH/8)),
[x] "r" ((uint8_t)x)
: :
); );
uint8_t data = sBuffer[row_offset] | bit;
if (color) { if (!(color & _BV(0))) data ^= bit;
sBuffer[row_offset] |= bit; sBuffer[row_offset] = data;
} else {
sBuffer[row_offset] &= ~ bit;
}
} }
uint8_t Arduboy2Base::getPixel(uint8_t x, uint8_t y) uint8_t Arduboy2Base::getPixel(uint8_t x, uint8_t y)
@ -922,7 +903,7 @@ struct BitStreamReader
} }
if ((this->byteBuffer & this->bitBuffer) != 0) if ((this->byteBuffer & this->bitBuffer) != 0)
result |= (1 << i); // result |= bitshift_left[i]; result |= (1 << i);
this->bitBuffer <<= 1; this->bitBuffer <<= 1;
} }

View File

@ -449,7 +449,7 @@ class Arduboy2Base : public Arduboy2Core
* specified color. The values WHITE or BLACK can be used for the color. * specified color. The values WHITE or BLACK can be used for the color.
* If the `color` parameter isn't included, the pixel will be set to WHITE. * If the `color` parameter isn't included, the pixel will be set to WHITE.
*/ */
void drawPixel(int16_t x, int16_t y, uint8_t color = WHITE); static void drawPixel(int16_t x, int16_t y, uint8_t color = WHITE);
/** \brief /** \brief
* Returns the state of the given pixel in the screen buffer. * Returns the state of the given pixel in the screen buffer.