3 ms·

The right way to do this in C would be to define each type of pixel in its own file, along with processing functions, and use function pointers within the struc

The right way to do this in C would be to define each type of pixel in its own file, along with processing functions, and use function pointers within the structs for required processing. C++ classes are really just syntactic sugar for data organization. You can do the same type of organization in C, things just have to be more explicit. The benefit if using C is that you can use constructs where needed while trimming fat where its not.

wott 5y ago
Well, you don't really need to mimic OO to make that code cleaner and less error prone. Here's what he wrote in order to complain about it: if (mode == ambient) { // handle pixels as ambient... int steps = right.a.x - left.a.x; float dv = (right.a.ambientLight - left.a.ambientLight)/steps; float currentIntensity = left.a.ambientLight; for (i=left.a.x; i<right.a.x; i++) { WorkOnPixelAmbient(i, dv); currentIntensity+=dv; } } else if (mode == gouraud) { // handle pixels as gouraud... int steps = right.g.x - left.a.x; float dred = (right.g.red - left.g.red)/steps; float dgreen = (right.g.green - left.g.green)/steps; float dblue = (right.g.blue - left.g.blue)/steps; float currentRed = left.g.red; float currentGreen = left.g.green; float currentBlue = left.g.blue; for (j=left.g.x; i<right.g.x; j++) { WorkOnPixelGouraud(j, currentRed, currentBlue, currentGreen); currentRed+=dred; currentGreen+=dgreen; currentBlue+=dblue; } Nobody should do that. We use a temporary variable of the correct type. For example this way (but it could also be done with pointers to access directly the union variable): switch(mode) { case AMBIENT: { PixelDataAmbient l = left.a; PixelDataAmbient r = right.a; int steps = r.x - l.x; float dv = (r.ambientLight - l.ambientLight)/steps; /* ... */ break; } case GOURAUD: { PixelDataGouraud l = left.g; PixelDataGouraud r = right.g; int steps = r.x - l.x; /* ... */ break; } /* ... */ default: show_error("Illegal mode"); } There is no need to cart all the .a. and .g. around and clutter the code. And there is no possibility to accidentally access the wrong fields the wrong way: trying to access l.ambientLight in the second block (GOURAUD) will fail at compilation since a PixelDataGouraud doesn't have an ambientLight field. It is also not possible to do PixelDataGouraud l = left.a; since left.a is not a PixelDataGouraud. Much cleaner, much more readable and avoids most of the possible errors he complains about.
- ActorNightly 5y ago
  You can definitely do that, however in practice, separating out by files makes the code very easy to test, since you can validate functionality per pixel type, and then validate your outer loop by passing fake objects with correct functions. You can also have internal workings with static functions/variables that you can use as helpers, but invisible to outside code and not declared in header files.

wott 5y ago

Well, you don't really need to mimic OO to make that code cleaner and less error prone. Here's what he wrote in order to complain about it: if (mode == ambient) { // handle pixels as ambient... int steps = right.a.x - left.a.x; float dv = (right.a.ambientLight - left.a.ambientLight)/steps; float currentIntensity = left.a.ambientLight; for (i=left.a.x; i<right.a.x; i++) { WorkOnPixelAmbient(i, dv); currentIntensity+=dv; } } else if (mode == gouraud) { // handle pixels as gouraud... int steps = right.g.x - left.a.x; float dred = (right.g.red - left.g.red)/steps; float dgreen = (right.g.green - left.g.green)/steps; float dblue = (right.g.blue - left.g.blue)/steps; float currentRed = left.g.red; float currentGreen = left.g.green; float currentBlue = left.g.blue; for (j=left.g.x; i<right.g.x; j++) { WorkOnPixelGouraud(j, currentRed, currentBlue, currentGreen); currentRed+=dred; currentGreen+=dgreen; currentBlue+=dblue; } Nobody should do that. We use a temporary variable of the correct type. For example this way (but it could also be done with pointers to access directly the union variable): switch(mode) { case AMBIENT: { PixelDataAmbient l = left.a; PixelDataAmbient r = right.a; int steps = r.x - l.x; float dv = (r.ambientLight - l.ambientLight)/steps; /* ... */ break; } case GOURAUD: { PixelDataGouraud l = left.g; PixelDataGouraud r = right.g; int steps = r.x - l.x; /* ... */ break; } /* ... */ default: show_error("Illegal mode"); } There is no need to cart all the .a. and .g. around and clutter the code. And there is no possibility to accidentally access the wrong fields the wrong way: trying to access l.ambientLight in the second block (GOURAUD) will fail at compilation since a PixelDataGouraud doesn't have an ambientLight field. It is also not possible to do PixelDataGouraud l = left.a; since left.a is not a PixelDataGouraud. Much cleaner, much more readable and avoids most of the possible errors he complains about.

ActorNightly 5y ago
You can definitely do that, however in practice, separating out by files makes the code very easy to test, since you can validate functionality per pixel type, and then validate your outer loop by passing fake objects with correct functions. You can also have internal workings with static functions/variables that you can use as helpers, but invisible to outside code and not declared in header files.