Structs within structs, and how to assign them?

thatrabbit

Hello. I have a structure:

#SelectExpand
 1typedef struct ColorRing {
 2    ALLEGRO_COLOR color;
 3    struct ColorRing* next;
 4} ColorRing;

And a function to initialize that structure:

#SelectExpand
  1ColorRing* create_color_ring_blue() {
  2    int scale       = 8;
  3    ColorRing* root = (ColorRing*)malloc(sizeof(ColorRing) * scale);
  4
  5    for(int i = 0; i < scale; i++) {
  6        root->color = al_map_rgb(0, 0, 128);
  7
  8        // This always prints 0. Why?
  9        printf("Color: %i\n", root->color);
 10
 11        if(i + 1 < scale) {
 12            root->next = root + 1;
 13            root++;
 14        } else {
 15            root->next = root - (scale - 1);
 16        }
 17    }
 18
 19    return root - (scale - 1);
 20}

This is what I don't understand:

#SelectExpand
 1// This always prints 0. Why?
 2printf("Color: %i\n", root->color);

How do I assign the output of al_map_rgb to that structure's member?

-- Edit

Seems al_map_rgb always returns 0 via printf.

In any event, the desired usage of the color is here:

#SelectExpand
 1ColorRing* cr = create_color_ring_blue();
 2al_draw_filled_rectangle(..., cr->color);

This only ever produces black.

Neil Roy

Try using al_unmap_rgb() (https://www.allegro.cc/manual/5/al_unmap_rgb). It does the reverse of al_map_rgb() and retrieves the three colours (RGB) for you.

Any function that takes an ALLEGRO_COLOR value you can use your color value directly. But if you want to retrieve the stored RGB values, you will need to call al_unmap_rgb() to retrieve them.

thatrabbit

NiteHackr said:

But if you want to retrieve the stored RGB values, you will need to call al_unmap_rgb() to retrieve them.

Nay, sorry, that's not my goal. I edited my post to make my intended-usage more clear.

Thank you for your reply.

Neil Roy

Ah, okay, than with your desired usage, it will work just fine as that takes an ALLEGRO_COLOR.

Edgar Reynaldo

You must not call most allegro functions (like al_map_rgb) before al_init.

thatrabbit

Edgar Reynaldo said:

You must not call most allegro functions (like al_map_rgb) before al_init.

*sigh* Okay. That works. You've obviously seen this before?

I feel mildly similar to when I first wrote this:

#SelectExpand
 1// Call this BEFORE we call any bitmap-related functions.
 2// Otherwise, we get shitty performance. Like really shitty.
 3//
 4display = al_create_display(192, 192);

Thank you, Edgar.

Edgar Reynaldo

Meh, just consider it an initiation rite. Everyone always misses stuff like that. You wouldn't think al_map_rgb would matter, but they cache values, so it does.

Welcome to the club. May I suggest you join our little -TINS- KrampusHack this year?

bamccaig

thatrabbit said:

Hello. I have a structure:

#SelectExpand
  1ColorRing* create_color_ring_blue() {
  2    int scale       = 8;
  3    ColorRing* root = (ColorRing*)malloc(sizeof(ColorRing) * scale);
  4
  5    for(int i = 0; i < scale; i++) {
  6        root->color = al_map_rgb(0, 0, 128);
  7
  8        // This always prints 0. Why?
  9        printf("Color: %i\n", root->color);
 10
 11        if(i + 1 < scale) {
 12            root->next = root + 1;
 13            root++;
 14        } else {
 15            root->next = root - (scale - 1);
 16        }
 17    }
 18
 19    return root - (scale - 1);
 20}

Just to nitpick, this would be easier to understand if you left root pointing to the first element, and used a new variable it or iterator (or whatever) to move through the array. It's also easier to split the special last case out of the loop.

#SelectExpand
  1ColorRing * create_color_ring_blue() {
  2    const int scale = 8;
  3    ColorRing * root = (ColorRing *)malloc(sizeof(ColorRing) * scale);
  4
  5    if (root == NULL) return NULL;
  6
  7    ALLEGRO_COLOR blue = al_map_rgb(0, 0, 128);
  8
  9    char r, g, b;
 10    al_unmap_rgb(blue, &r, &g, &b);
 11    fprintf(stderr, "Color: %d %d %d\n", (int)r, (int)g, (int)b);
 12
 13    ColorRing * last = &root[scale - 1];
 14
 15    for(ColorRing * it = root; it < last; it++) {
 16        it->color = blue;
 17        it->next = it + 1;
 18    }
 19
 20    last->color = blue;
 21    last->next = root;
 22
 23    return root;
 24}

Arthur Kalliokoski

Edgar Reynaldo said:

Everyone always misses stuff like that.

I've been messing with DOS stuff again, and was fiddling with a little program to show the colors in CGA video mode 4. I couldn't get it to work right for 6 HOURS (although in my defense I couldn't seem to catch it in GDB and none of the windowed debuggers I have will work for more than 2 minutes)

It was supposed to show the top 25% as one color (black), then each additional quarter of the screen showed the other 3 colors, but this showed the top 50% as black.

Can you spot the problem?

#SelectExpand
  1#include <stdio.h>
  2#include <stdlib.h>
  3#include <string.h>
  4#include <time.h>
  5#include <dos.h>
  6
  7unsigned char prop(unsigned char ch)
  8{
  9  return ch + (ch<<2) + (ch << 4) + (ch << 6);
 10}
 11
 12int main(void)
 13{
 14
 15  //char far *p  = 0xB8000000;  //For the 16 bit Watcom compiler, but
 16  //char far *p2 = 0xBA000000;  //it doesn't work right no matter which compiler
 17
 18  char *p  = 0xB8000;  //CGA graphics are "interleaved" so odd numbered
 19  char *p2 = 0xBA000;  //scanlines are 8K higher in memory
 20
 21  int i;
 22  unsigned char c;
 23
 24
 25  c = prop(0);
 26  printf("%X\n",c);
 27  c = prop(1);
 28  printf("%X\n",c);
 29  c = prop(2);
 30  printf("%X\n",c);
 31  c = prop(3);
 32  printf("%X\n",c);
 33
 34  getch();
 35
 36  _asm pusha
 37  _asm mov ax,4;
 38  _asm int 10h;
 39  _asm popa
 40
 41  c = prop(0);
 42  for(i=0;i<2000;i++)
 43  {
 44    p[i] = c;
 45    p2[i] = c;
 46  }
 47
 48  c = prop(1);
 49  for(;i<4000;i++);
 50  {
 51    p[i] = c;
 52    p2[i] = c;
 53  }
 54
 55  c = prop(2);
 56  for(;i<6000;i++)
 57  {
 58    p[i] = c;
 59    p2[i] = c;
 60  }
 61
 62  c = prop(3);
 63  for(;i<8000;i++)
 64  {
 65    p[i] = c;
 66    p2[i] = c;
 67  }
 68
 69  getch();
 70
 71  _asm pusha
 72  _asm mov ax,3;
 73  _asm int 10h;
 74  _asm popa
 75
 76  return 0;
 77}

Neil Roy

Arthur Kalliokoski said:

Can you spot the problem?

Hmmm... never worked with CGA mode before, only VGA. The only thing I can think of is your character pointers rather than unsigned character pointers?

You make me want to mess with DOS again. I miss that. What are you using to compile this with? I used to use DJGPP to compile with when I first learned C under DOS. I still have ALL my old source code as I'm a packrat and keep even the stupid little things, including many code examples I got from Allegro people back in the '90s!

Arthur Kalliokoski

NiteHackr said:

never worked with CGA mode before, only VGA

Broad hint: The problem has nothing to do with hardware or CGA, just a coding error. I went so far as to trying it in system memory on Linux and examined the memory with a debugger, which still had the problem.

Or if you wanna cheat...

My treacherous little fingers muscle memory went and snuck in the semicolon at the end of that for(;;) loop on line 49 so the body of the loop doesn't get executed

As to the other questions, I was tired of OpenGL inconsistencies etc. so I decided to just blit a memory buffer to a window with graphic primitives similar to Allegro 4, and one day thought "I wonder how slow it would be in DOS" although CGA resolution is too small for just about any sort of interface beyond a pixel-y game. I'm using Watcom and nasm in case I feel like giving something to the freedos crowd as those are their preferences.

I've hijacked this thread far enough.

RmBeer2

If you want to visualize something use %x

Neil Roy

Ah!!! I don't know how I didn't spot that, but I have made the same mistake. In fact, it is probably my most common mistake to make! I had to read your spoiler to see it, but now it seems so obvious. Such a simple mistake, but a nasty one and difficult to find.

I can see how a debugger wouldn't catch that as it is still valid code... though I would think that a decent IDE would warn you of it given the code following it. Is there a flag to warn of such things? I know I would turn it on if there was.

Peter Hull

Apple's compiler gives this (even with no 'warning' flags applied)

$ gcc ak.c

ak.c:49:19: warning: for loop has empty body [-Wempty-body]
  for(;i<4000;i++);

(though obviously none of the rest of it compiles anyway)

So it's definitely possible for the compiler to detect this - maybe you just need a newer version?

Arthur Kalliokoski

Peter Hull said:

Apple's compiler gives this (even with no 'warning' flags applied)

I don't have the original anymore (just a one-off test) but

#SelectExpand
  1#include <stdio.h>
  2
  3#define LIMIT 1000
  4
  5unsigned int array[LIMIT];
  6
  7int main(void)
  8{
  9  int i;
 10
 11  for(i=0;i<10;i++);
 12    array[LIMIT] = i;
 13
 14  //just to make sure the whole thing doesn't get optimized away
 15  for(i=0;i<LIMIT;i++)
 16    printf("%d\n",array[i]);
 17
 18  return 0;
 19}

run through Slackware current gcc version 5.5.0 gave an error as
"warning: array subscript is above array bounds [-Warray-bounds]"
with -O2 -Wall options. Changing line 12 to read
array[4] = i;
got it to compile with no warnings or errors

The latest DJGPP has gcc version 9.3.0 (!) which gave these warnings

t.c: In function 'main':
t.c:12:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
   12 |  for(i=0;i<10;i++);
      |  ^~~
t.c:13:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'for'
   13 |   array[LIMIT] = i;
      |   ^~~~~
t.c:13:8: warning: array subscript 1000 is above array bounds of 'unsigned int[1000]' [-Warray-bounds]
   13 |   array[LIMIT] = i;
      |   ~~~~~^~~~~~~
t.c:6:14: note: while referencing 'array'
    6 | unsigned int array[LIMIT];
      |              ^~~~~

OTOH, I seem to remember an old ALLEGRO.TXT as saying something about such a loop being used for a delay
"If the compiler complains, try changing it to
for(i=0;i<1000;i++) {};
to beat the compiler into submission."

Neil Roy

array[LIMIT] is definitely out of bounds if you set the LIMIT to 1000, then use this, the maximum is only 999, so this should be array[LIMIT-1].

But anyhow, yeah, for loops were common for delays, which is why I originally seen it as valid (even if poor) code. I would expect a warning when it is followed with braces and/or indented code though.

Nice to see DJGPP is still going.

You're making me want to fire up some old DOS projects again.

DanielH

Also, did you notice the semicolon on line 11?

Thread #618294. Printed from Allegro.cc