c++x86osdevdouble-buffering

How do I fix my double-buffer not drawing to screen?


I'm working on a small operating system, and I ran into some screen-tearing, so I'm working on double-buffering to solve that. I'm just now running into the issue that now nothing prints to screen after revamping my rendering methods. A list of a few things that could be the issue, though I'm really not sure:

  1. I did a lot of sketchy things to get the void dst and src to be compatible with u8 d and s in memcpy function
  2. I implemented (or should I say copied from an external source) the double buffering bit, and there might be something I forgot to convert to the new, better system
  3. A typo (unlikely)

Here's my C++ code:

typedef unsigned char uint8_t;
typedef unsigned char u8;
typedef unsigned short uint16_t;
typedef unsigned int u32;
typedef u32 size_t;
#define SCREEN_WIDTH 320
#define SCREEN_HEIGHT 200
#define SCREEN_SIZE (SCREEN_WIDTH * SCREEN_HEIGHT)
#define FPS 30
#define PIT_HERTZ 1193131.666
#define CLOCK_HIT (int)(PIT_HERTZ/FPS)
static uint8_t *BUFFER = (uint8_t *) 0xA0000;

// double buffers
uint8_t _sbuffers[2][SCREEN_SIZE];
uint8_t _sback = 0;

#define CURRENT (_sbuffers[_sback])
#define SWAP() (_sback = 1 - _sback)

#define screen_buffer() (_sbuffers[_sback])

#define screen_set(_p, _x, _y)\
    (_sbuffers[_sback][((_y) * SCREEN_WIDTH + (_x))]=(_p))

const unsigned char font[128-32][8] = {
    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},   // U+0020 (space)
\*deleted to help with length and readability of code...*\
    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}    // U+007F
};

static inline void *memcpy(void *dst, const void *src, size_t n) {
    u8 *d = (u8*)(&dst);
    const u8 *s = (u8*)(&src);

    while (n-- > 0) {
        *d++ = *s++;
    }

    return d;
}

void screen_swap() {
    memcpy(BUFFER, &CURRENT, SCREEN_SIZE);
    SWAP();
}

static inline void outb(uint16_t port, uint8_t val)
{
    asm volatile ( "outb %0, %1" : : "a"(val), "Nd"(port) );
}

static inline uint8_t inb(uint16_t port)
{
    uint8_t ret;
    asm volatile ( "inb %1, %0"
                   : "=a"(ret)
                   : "Nd"(port) );
    return ret;
}

unsigned read_pit(void) {
    unsigned count = 0;
 
    // al = channel in bits 6 and 7, remaining bits clear
    outb(0x43,0b0000000);
 
    count = inb(0x40);          // Low byte
    count |= inb(0x40)<<8;      // High byte
 
    return count;
}
 
void draw_char(char c, int x, int y, unsigned char color)
{
    const unsigned char *glyph = font[(int)c-32];
 
    for(int cy=0;cy<8;cy++){
        for(int cx=0;cx<8;cx++){
            if(((int)glyph[cy]&(1<<cx))==(1<<cx)){
                screen_set(color,x+cx,y+cy);
            }
        } 
    }
}

void draw_string(const char * s, int x, int y, unsigned char color) {
    int i = 0;
    while(s[i] != false) {
        draw_char(s[i],x+(i*8),y,color);
        i++;
    }
}

void draw_rect(int pos_x, int pos_y, int w, int h, unsigned char color) {
    for(int y = 0; y<h; y++) {
        for(int x = 0; x<w; x++) {
            screen_set(color,x+pos_x,y+pos_y);
        }
    }
}

static void render() {
    //draw_rect(0,0,SCREEN_WIDTH,SCREEN_HEIGHT,0);
    draw_string("Hello, reader. This is written text.", 10, 18, 15);
    draw_string("If this is displayed, my code works.", 10, 10, 15);
}

extern "C" void main(){
    int clock = 0;
    while(true) {
        clock++;
        if(read_pit()!= 0 && clock == CLOCK_HIT) {
            clock = 0;
            render();
            screen_swap();
        }
    }

    return;
}

Solution

  • I did a lot of sketchy things to get the void dst and src to be compatible with u8 d and s in memcpy function

    Yes, you did. And often that's an indication you're doing something wrong. Let's look at it:

    static inline void *memcpy(void *dst, const void *src, size_t n) {
        u8 *d = (u8*)(&dst);
        const u8 *s = (const u8*)(&src);
    
        while (n-- > 0) {
            *d++ = *s++;
        }
    
        return d;
    }
    

    Here, you take the address of your pointer variables (instead of the pointer value itself). So that's a void**, which you then cast to a u8*. When you dereference that, you are then looking at individual bytes of the memory that holds this pointer. That's clearly incorrect.

    Another thing that's weird is your function returns the value of d+n. This is not how the standard memcpy function works. It will always return dst. So, consider doing that if you really want to have a function with standard behavior.

    This is what you should have done:

    static inline void *memcpy(void *dst, const void *src, size_t n)
    {
        u8 *d = (u8*)dst;
        const u8 *s = (const u8*)src;
    
        while (n-- > 0) {
            *d++ = *s++;
        }
        return dst;
    }
    

    You're also invoking it strangely:

    memcpy(BUFFER, &CURRENT, SCREEN_SIZE);
    

    Now, technically &CURRENT works okay because your CURRENT macro points to an array. But taking the address of an array is something you should avoid. Let the compiler convert the array to a pointer for you:

    memcpy(BUFFER, CURRENT, SCREEN_SIZE);
    

    Here's an alternative implementation that avoids decrementing n every loop:

    static inline void *memcpy(void *dst, const void *src, size_t n)
    {
        u8 *d = (u8*)dst;
        const u8 *s = (const u8*)src;
        const u8 *s_end = s + n;
    
        while (s != s_end) {
            *d++ = *s++;
        }
        return dst;
    }
    

    If you're using a C compiler, also consider also adding restrict keywords because src and dst do not overlap.