Conversation
vdu_plot function rewritten - it now splits the command byte into separate “mode” and “operation”. code is structured to allow for all Acorn GXR VDU operations to be supported, except for sprites, in preparation for implementation (where practical) relative and absolute commands are now supported for all operations all existing operations are supported, plus in this commit line variants that omit first/last points, and filled circles have been added, and circle sizes fixed
make sure when setting origin and pushing points our values are correct plus move graphics cursor to 0,0
| fabgl::PaintOptions tpo; // Text paint options | ||
|
|
||
| Point p1, p2, p3; // Coordinate store for plot | ||
| Point rp1, rp2, rp3; // Relative coordinates store for plot |
There was a problem hiding this comment.
tracking 3 relative points is not really needed - only the most recent relative point is currently ever used. for now I left them in as they're not particularly costly and they may prove useful in the future
| void setGraphicsOptions(uint8_t mode) { | ||
| auto colourMode = mode & 0x03; |
There was a problem hiding this comment.
the mode value sent here is the bottom 3 bits of the plot code - a number in the range 0-7.
when it comes to picking colours we only really care about the bottom 2 bits, as the top bit is used to indicate whether coordinates are relative or absolute
| canvas->setPenColor(gfg); | ||
| canvas->setBrushColor(gfg); | ||
| } break; | ||
| case 2: break; // logical inverse colour (not suported) |
There was a problem hiding this comment.
the "logical inverse" colour is not something we can really support. There is a "not" painting option inside fab-gl, but it only works for a highly restricted range of operations (the documentation says "Implemented only for straight lines and non-filled rectangles").
to be honest, I had initially understood this to mean it only works for horizontal or vertical lines - and as that is very highly restrictive I wrote the code so there was no support for this mode at all. having looked at the code it is any straight line, and so therefore it is slightly more useful than I'd thought, so will likely add in support for inverse soon.
once that support is added, we'll need to take some care to document quite how limited the "inverse" support is.
| void plotLine(bool omitFirstPoint = false, bool omitLastPoint = false) { | ||
| RGB888 firstPixelColour; | ||
| RGB888 lastPixelColour; | ||
| if (omitFirstPoint) { | ||
| firstPixelColour = canvas->getPixel(p2.X, p2.Y); | ||
| } | ||
| if (omitLastPoint) { | ||
| lastPixelColour = canvas->getPixel(p1.X, p1.Y); | ||
| } | ||
| canvas->lineTo(p1.X, p1.Y); | ||
| if (omitFirstPoint) { | ||
| canvas->setPixel(p2, firstPixelColour); | ||
| } | ||
| if (omitLastPoint) { | ||
| canvas->setPixel(p1, lastPixelColour); | ||
| } | ||
| } |
There was a problem hiding this comment.
grabbing the first/last pixel, drawing the line, and replacing it back on the screen is the simplest way to implement those line variants
this obviously isn't the most efficient approach, but doing this the "right" way either requires modifications to the line drawing routines inside fab-gl, or working out different start/end points (which may result in slightly different lines being drawn anyway). the cost here is pretty negligible
| pushPoint(0,0); | ||
| pushPoint(0,0); | ||
| pushPoint(0,0); | ||
| moveTo(); |
There was a problem hiding this comment.
adding the moveTo here means that our first point is at our (current) 0,0... before this our initial point on mode change would be at the top left of the screen.
the other commands here are moved later to ensure that 0,0 is correctly calculated, as changing screen mode can change width and height
| plotLine(true, true); | ||
| break; | ||
| case 0x40: // point | ||
| plotPoint(); |
There was a problem hiding this comment.
to support "invert" we can't just read the pixel, NOT it, and write it back to screen, as the read pixel is RGB888 format, and the inverted version of that may not equate to it's logically inverse colour.
instead we should use canvas->invertRect().
| case 0xC0: // ellipse outline | ||
| case 0xC8: // ellipse fill | ||
| // fab-gl's ellipse isn't compatible with BBC BASIC | ||
| debug_log("plot ellipse not implemented\n\r"); | ||
| break; |
There was a problem hiding this comment.
fab-gl does have an "ellipse" command, but it's a simplistic flattened circle thing, whilst Acorn's ellipse is a more complex thing
| void plotCircle(bool filled = false) { | ||
| auto size = 2 * sqrt(rp1.X * rp1.X + rp1.Y * rp1.Y); | ||
| if (filled) { | ||
| canvas->fillEllipse(p2.X, p2.Y, size, size); | ||
| } else { | ||
| canvas->drawEllipse(p2.X, p2.Y, size, size); | ||
| } | ||
| } |
There was a problem hiding this comment.
the previous "relative" variant of circle wasn't properly calculating the size
one thing to note here is that most Agon machines are being used on a 16:9 screen, but our screen modes are essentially use a 4:3 aspect ratio, and thus have non-square pixels. this means circles are squished when they're drawn.
this can be compensated for by multiplying the height (second size argument) by 1.3333333 (or dividing the width by that amount).
it feels wrong to just blindly squish all circles, as that would make them incompatible with Acorn's circle plots. however it also feels wrong that we're not providing a way to draw circles that will appear round on a 16:9 screen.
there's three potential ways to address this. firstly we could expose fab-gl's "squished circle" ellipse variant under a set of PLOT numbers that's not already assigned (Acorn never used the range &D8-E7 - this is my least favourite option). secondly we could implement Acorn compatible "ellipse" commands, thus allowing stretched circles to be drawn within the existing PLOT numbers, and document examples of how to draw round circles. finally we could extend the "logical screen scaling" VDU command to look for a second bit to indicate "16:9 circle compensation" (VDU 23,0,&C0,3 would compensate circles within the logical coordinate system, and VDU 23,0,&C0,2 would compensate circles for native screen coordinates)
Update VDU plot support to handle more options and operations.
Plot codes extended to support as much of Acorn's Graphics eXtension ROM (GXR) as is practical with the capabilities of fab-gl.
Line variants that omit first/last points, rectangle, parallelogram, filled circles, and rectangle copy/move plot codes are all now supported.
All plot variants except for the "logical inverse colour" versions are supported.
Unfortunately as well as the "inverse" variants, "line fill left/right", "dot-dash line", "flood", arc, segment, sector and ellipse commands cannot currently be handled. Supporting them would only be possible with changes to the underlying fab-gl/vdp-gl code.