vanvught / rpidmx512

Orange Pi DMX512 / RDM / MIDI / OSC / Art-Net / WS28xx / L6470 / Stepper / TLC59711 / PCA9685 / Servo / PWM / TCNet / SMPTE / RDMNet / LLRP / GD32 / GigaDevice / Raspberry Pi
http://www.orangepi-dmx.org/
MIT License
396 stars 108 forks source link

Experimental #141

Closed hippyau closed 4 years ago

hippyau commented 4 years ago

Hey Arjan!

I am messing around with my oPi One now, and HDMI.

I have the Nuklear GUI (link), is it a very light header-only GUI with no stdlib dependencies, and has a back-end rendered to draw to a frame buffer. I don't know why... maybe just to make a pretty display for some monitor firmware, just experimenting and don't expect to you to merge :)

I made a new scratch-h3-nuklear and I have it compiling (had to disable all warning in rules.mk to get there!), but when linking I get crazy stuff... Wondering if you might know why abort and memcpy is undefined here and the unwind stuff? No stress, just experiments... be cool if it works :)

$TARGET [lib_h3/libarm.a ]
arm-none-eabi-ar: creating lib_h3/libarm.a
/usr/share/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7ve+simd/hard/libgcc.a(pr-support.o): In function `_Unwind_GetDataRelBase':
pr-support.c:(.text.unlikely+0x2): undefined reference to `abort'
build_h3/firmware/main.o: In function `nk_style_from_table':
main.cpp:(.text+0xe3d8): undefined reference to `memcpy'
main.cpp:(.text+0xe648): undefined reference to `memcpy'
main.cpp:(.text+0xe6f4): undefined reference to `memcpy'
main.cpp:(.text+0xe708): undefined reference to `memcpy'
main.cpp:(.text+0xe718): undefined reference to `memcpy'
build_h3/firmware/main.o:main.cpp:(.text+0xe81c): more undefined references to `memcpy' follow
build_h3/firmware/main.o: In function `notmain':
main.cpp:(.text+0x26a5c): undefined reference to `__cxa_end_cleanup'
build_h3/firmware/main.o:(.ARM.extab+0x2c4): undefined reference to `__gxx_personality_v0'
/usr/share/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7ve+simd/hard/libgcc.a(unwind-arm.o): In function `get_eit_entry':
unwind-arm.c:(.text+0x10c): undefined reference to `__exidx_end'
unwind-arm.c:(.text+0x110): undefined reference to `__exidx_start'
unwind-arm.c:(.text+0x114): undefined reference to `__exidx_end'
unwind-arm.c:(.text+0x118): undefined reference to `__exidx_start'
/usr/share/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7ve+simd/hard/libgcc.a(unwind-arm.o): In function `unwind_phase2':
unwind-arm.c:(.text+0x1e6): undefined reference to `abort'
/usr/share/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7ve+simd/hard/libgcc.a(unwind-arm.o): In function `unwind_phase2_forced':
unwind-arm.c:(.text+0x23e): undefined reference to `memcpy'
unwind-arm.c:(.text+0x26e): undefined reference to `memcpy'
/usr/share/gcc-arm-none-eabi-9-2020-q2-update/bin/../lib/gcc/arm-none-eabi/9.3.1/thumb/v7ve+simd/hard/libgcc.a(unwind-arm.o): In function `__gnu_Unwind_Resume':
unwind-arm.c:(.text+0x37c): undefined reference to `abort'
unwind-arm.c:(.text+0x380): undefined reference to `abort'
../h3-firmware-template/Rules.mk:221: recipe for target 'build_h3/main.elf' failed
make: *** [build_h3/main.elf] Error 1
vanvught commented 4 years ago

Hi Hippy,

Have you checked https://github.com/Immediate-Mode-UI/Nuklear/blob/562220749c621b033bc05e621c36e8964e645909/src/HEADER with

/// !!! WARNING /// The following flags will pull in the standard C library: /// - NK_INCLUDE_DEFAULT_ALLOCATOR /// - NK_INCLUDE_STANDARD_IO /// - NK_INCLUDE_STANDARD_VARARGS

I have no standard library. I have just implemented the standard C-functions which are needed for my firmware.

The errors:

unwind-arm.c:(.text+0x10c): undefined reference to `__exidx_end'
unwind-arm.c:(.text+0x110): undefined reference to `__exidx_start'
unwind-arm.c:(.text+0x114): undefined reference to `__exidx_end'
unwind-arm.c:(.text+0x118): undefined reference to `__exidx_start'

can be solved with adding:

    __exidx_start = .;
    .ARM.exidx   : { *(.ARM.exidx* .gnu.linkonce.armexidx.*) } >sram
    __exidx_end = .;

in https://github.com/vanvught/rpidmx512/blob/experimental/h3-firmware-template/memmap

Please be aware that my code does not have any third party code (other than FatFS, as rewriting/debuggin FAT is time consuming;). As I want to be 100% responsible for the code on my Github, I am not going to add any third party headers (or code).

Greetings, Arjan

vanvught commented 4 years ago

had to disable all warning in rules.mk to get there!),

That's also a major reason why not having third party code. All the C code must be able to compile with:

-Wall -Werror -Wpedantic -Wextra -Wunused -Wsign-conversion

and additional for C++:

std=c++11 -Wuseless-cast -Wold-style-cast -Wnon-virtual-dtor -Wnull-dereference -fno-rtti -fno-exceptions -fno-unwind-tables

vanvught commented 4 years ago

Please also be aware that currently the framebuffer is only available when booting from SDCard. The SPI boot loader does not have HDMI enabled.

As you can see here https://github.com/vanvught/rpidmx512/blob/experimental/scratch-h3-c%2B%2B/firmware/main.cpp#L127 I am working on my own HDMI code and simple framebuffer support for Orange Pi One (Allwinner H3).

vanvught commented 4 years ago

I noticed that you are using the capital #defines in main. However you should use the lower volatile variables instead -> https://github.com/vanvught/rpidmx512/blob/master/lib-h3/device/fb/fb.c#L73

and also as reference here -> https://github.com/vanvught/rpidmx512/blob/master/lib-hal/src/h3/console_fb.c#L53

This is just all a temporally implementation; when I have my own HDMI code, then we are back to FB_WIDTH, FB_HEIGHT and FB_PITCH. With a fixed, well defined, framebuffer address.

vanvught commented 4 years ago

Hi Hippy,

Please will you remove the files: Rules.mk and memmap from the commit? This code should run without modifications to these global configuration files.

Drawing(void) { s_pThis = this; } The .cpp file is missing the initializer for the static Drawing *s_pThis

I am not sure why these must be static:

static int32_t fbW(void) { return static_cast<int32_t>(fb_width); }
static int32_t fbH(void) { return static_cast<int32_t>(fb_height); }

As the fb_width and the fb_height are always unsigned, the fbW and fbH can be unsigned as well; removing the unnecessarily static_cast.

This .cpp file is not H3 specific. Hence the h3 files should not be included.

#include "h3.h"
#include "h3/console_fb.h"

Same here

void Drawing::pixel(int32_t x, int32_t y, uint32_t c) {
         console_putpixel(static_cast<uint32_t>(x), static_cast<uint32_t>(y), c);
 }

x, y are always unsigned. And we can remove the static_cast.

With Modern C++11 there are no #define , please convert the below to a private method or template.

#define SWAP(x,y) do \
     { unsigned char swap_temp[sizeof(x) == sizeof(y) ? static_cast<signed>(sizeof(x)) : -1]; \
      memcpy(swap_temp,&y,sizeof(x)); \
      memcpy(&y,&x,       sizeof(x)); \
      memcpy(&x,swap_temp,sizeof(x)); \
     } 

There are several goto's. That really cannot happen in Modern C++ ;)

Thanks, Arjan

hippyau commented 4 years ago

Hey Arjan,

Thanks :) Yep I gave up on Nuklear, I got close but decided I just don't need 50K lines of code to knock up a display :)

So I've added these drawing commands, but this is not tested yet and just for starting. Thanks for the code review and I'm learning more and more every time.

I'll make those changes you suggested and hopefully draw something soon :)

hippyau commented 4 years ago

Hey Arjan,

This is driving my a little crazy, mostly because I don't know what I am doing :) I've f**ked my git again, but i'll cancel my request and do again once i'm feeling confident i'm getting close.

I have borrowed the code from here to draw various lines, and because the x2 can be less than x1, it needs to be int not unsigned int. So have to cast to keep parameters as uint32_t?

I have modified to this, it works, but I don't think how you would do it... 2020-08-11_22_48_40

void Drawing::line(uint32_t x1, uint32_t y1, uint32_t x2, uint32_t y2, uint32_t p, uint32_t pattern)
    {
        int x, y, dx, dy, dx1, dy1, px, py, xe, ye, i;

        int xx1 = static_cast<int>(x1);
        int yy1 = static_cast<int>(y1);
        int xx2 = static_cast<int>(x2);
        int yy2 = static_cast<int>(y2);

        dx = xx2 - xx1; dy = yy2 - yy1;

        auto rol = [&](void) { pattern = (pattern << 1) | (pattern >> 31); return pattern & 1; };

        // straight lines idea by gurkanctn
        if (dx == 0) // Line is vertical
        {
            if (yy2 < yy1) swap(yy1, yy2);
            for (y = yy1; y <= yy2; y++) if (rol()) pixel(static_cast<uint32_t>(xx1), static_cast<uint32_t>(y), p);
            return;
        }

        if (dy == 0) // Line is horizontal
        {
            if (xx2 < xx1) swap(xx1, xx2);
            for (x = xx1; x <= xx2; x++) if (rol()) pixel(static_cast<uint32_t>(x), static_cast<uint32_t>(yy1), p);
            return;
        }

        // Line is Funk-aye
        dx1 = abs(dx); dy1 = abs(dy);
        px = 2 * dy1 - dx1; py = 2 * dx1 - dy1;
        if (dy1 <= dx1)
        {
            if (dx >= 0)
            {
                x = xx1; y = yy1; xe = xx2;
            }
            else
            {
                x = xx2; y = yy2; xe = xx1;
            }

            if (rol()) pixel(static_cast<uint32_t>(x), static_cast<uint32_t>(y), p);

            for (i = 0; x < xe; i++)
            {
                x = x + 1;
                if (px < 0)
                    px = px + 2 * dy1;
                else
                {
                    if ((dx < 0 && dy < 0) || (dx > 0 && dy > 0)) y = y + 1; else y = y - 1;
                    px = px + 2 * (dy1 - dx1);
                }
                if (rol()) pixel(static_cast<uint32_t>(x), static_cast<uint32_t>(y), p);
            }
        }
        else
        {
            if (dy >= 0)
            {
                x = xx1; y = yy1; ye = yy2;
            }
            else
            {
                x = xx2; y = yy2; ye = yy1;
            }

            if (rol()) pixel(static_cast<uint32_t>(x), static_cast<uint32_t>(y), p);

            for (i = 0; y < ye; i++)
            {
                y = y + 1;
                if (py <= 0)
                    py = py + 2 * dx1;
                else
                {
                    if ((dx < 0 && dy < 0) || (dx > 0 && dy > 0)) x = x + 1; else x = x - 1;
                    py = py + 2 * (dx1 - dy1);
                }
                if (rol()) pixel(static_cast<uint32_t>(x), static_cast<uint32_t>(y), p);
            }
        }
    }

PS... thanks for putting up with amateur me :) Stay cool in the summer :)

vanvught commented 4 years ago

Stay cool in the summer :)

For some days already it is 34+ degrees Celsius. We are not used for that, hence there is no airco at home. And...it is now 28 degrees Celsius inside.

dx = xx2 - xx1; dy = yy2 - yy1;

From above I would conclude that d is delta. Where xx,yy are positive unsigned positions. The xx,yy can be uint32 And then the d's can be int32.

This

        {
            if (yy2 < yy1) swap(yy1, yy2);
            for (y = yy1; y <= yy2; y++) if (rol()) pixel(static_cast<uint32_t>(xx1), static_cast<uint32_t>(y), p);
            return;
        }

is very difficult to read.

Better

{
    if (yy2 < yy1) {
        swap(yy1, yy2)
    }
    for (y = yy1; y <= yy2; y++) {
        if (rol()) {
            pixel(static_cast<uint32_t>(xx1), static_cast<uint32_t>(y), p);
        }
    }

    return;
}

I am not sure if rol()returns a boolean. If not, then there must be a check.

This is tricky:

dx1 = abs(dx); dy1 = abs(dy);

Using the same d for delta, but this delta is absolute. So it can be an uint32_t

hippyau commented 4 years ago

For some days already it is 34+ degrees Celsius. We are not used for that, That's pretty much September thru June here, you never get used to it!!

This is very difficult to read.

Agreed, yes, I cleaned it up, but then started again and hadn't cleaned it up again yet.

Is casting CPU expensive? I preferred the old (int) style :)

I am very tired at the moment, so probably not looking at this properly.

I am looking at another line algorithm for suitability... seems much cleaner.

// Modified from  https://rosettacode.org/wiki/Bitmap/Bresenham%27s_line_algorithm#C
void bresenham(int x0, int y0, int x1, int y1) {
    int dx = abs(x1 - x0), sx = x0 < x1 ? 1 : -1;
    int dy = abs(y1 - y0), sy = y0 < y1 ? 1 : -1;
    int err = (dx > dy ? dx : -dy) / 2;

    while (setpixel(x0, y0), x0 != x1 || y0 != y1) {
        int e2 = err;
        if (e2 > -dx) { err -= dy; x0 += sx; }
        if (e2 <  dy) { err += dx; y0 += sy; }
    }
}

Good night, Cheers :)

hippyau commented 4 years ago

Hi Arjan, Basic drawing command, I have cleaned up a bit and made more efficient.

image

Seems to work well, yet to do any kind of performance measurements or benchmarking.

hippyau commented 4 years ago

Hey Arjan,

The above is a little bit odd ;)

Hehehe, yes I am a bit odd.

Okay, I was testing the Get() singleton thing, and it actually worked like this, but I understand this is not how it goes.

So in notmain() I can;

Drawing draw;
draw.line(....)

but then elsewhere I would Drawing::Get()->line(...) ?

All the Nuklear stuff should have been deleted in a previous commit, it's not in my fork anymore. Should I just close this and make a new fork with just the new stuff in it?

vanvught commented 4 years ago

So in notmain() I can;

Drawing draw; draw.line(....)

Yes, that is better.

but then elsewhere I would Drawing::Get()->line(...) ?

No, not really. When you are still in scope of the Drawing object then you can use draw

The ::Get()-> is used when accessing the unique object in another scope.

However, as this class does not have member variables, no allocation (hence no deletes, an empty destructor) , it is stateless and there is no synchronization issue (no task switching), the Drawing object can be instantiated multiple times; there can more instances of this object. No need for ::Get()->. Comparing with the Hardware object, or Network object, there can be just one. Hence the static ::Get()->