Conversation
c75cf4d to
1e5bdad
Compare
052bc00 to
222c38d
Compare
stevesims
left a comment
There was a problem hiding this comment.
lol - made all these comments and then didn't submit the review 😀
| class MultiBufferStream : public Stream { | ||
| public: | ||
| MultiBufferStream(std::vector<std::shared_ptr<BufferStream>, psram_allocator<std::shared_ptr<BufferStream>>> buffers); | ||
| MultiBufferStream(std::vector<std::shared_ptr<BufferStream>> buffers); |
There was a problem hiding this comment.
I believe there's no need for the allocator param here. these are shared pointers that are already constructed in PSRAM, so the allocator is redundant. its inclusion made some other code harder/more complex
| uint16_t currentBitmap = 0; // Current bitmap ID | ||
| std::unordered_map<uint16_t, std::shared_ptr<Bitmap>> bitmaps; // Storage for our bitmaps |
There was a problem hiding this comment.
bitmap ID now corresponds to a buffer ID
bitmaps are only usable tho if they have been created, since a buffer by itself has no info on width/height/format. this is achieved thru a single new "create bitmap from buffer" call.
changing to use buffers increases our potential number of bitmaps to 65534. supporting the increased number requires a single new "select bitmap" call.
all of the old/original bitmap VDU calls still work. for compatibility the original 8-bit bitmap identifiers map to buffers 64000-64255. this means the legacy "upload RGBA8888 bitmap" command will now store its bitmaps in buffers.
| // track which sprites may be using a bitmap | ||
| std::unordered_map<uint16_t, std::vector<uint8_t>> bitmapUsers; |
There was a problem hiding this comment.
we keep track of which sprites are using bitmaps. this allows us to ensure that sprites get disabled when the buffer for a bitmap is destroyed.
(some buffered commands API operations are destructive to their buffers, replacing them with newly allocated memory blocks, which would leave dangling pointers inside sprites. some operations however will edit buffers in-place allowing pointers to remain intact. this can, for instance, be used to mirror or rotate bitmaps that are being used in sprites without having to re-create the sprite, and they will change on next sprite refresh.)
| void resetSprites() { | ||
| hideAllSprites(); | ||
| for (auto n = 0; n < MAX_SPRITES; n++) { | ||
| clearSpriteFrames(n); | ||
| } | ||
| activateSprites(0); | ||
| // replace all the sprite objects | ||
| // for (auto n = 0; n < MAX_SPRITES; n++) { | ||
| // sprites[n] = Sprite(); | ||
| // } | ||
| waitPlotCompletion(); | ||
| } |
There was a problem hiding this comment.
I had noticed that when sprites had been used in one program, and then re-used in a different program, they weren't cleanly reset, even after resetSprites had been called. a sprite keeps a copy of the background underneath where it's shown and blits that back to screen (this is why sprites can flicker when drawing operations happen). that background isn't cleared within fab-gl when the number of active sprites is changed, so sprites can get left with "stale" backgrounds.
the approach here attempts to reset the background store. it seems to work, but I'm not convinced it's completely correct. the more proper way to resolve this is to adjust the handling within fab-gl
| // | ||
| void VDUStreamProcessor::bufferWrite(uint16_t bufferId) { | ||
| auto length = readWord_t(); | ||
| uint32_t VDUStreamProcessor::bufferWrite(uint16_t bufferId, uint32_t length) { |
There was a problem hiding this comment.
now accepts a length, allowing this call to be used by the bitmaps API
| if (bufferId == id) { | ||
| // this is our current buffer ID, so rewind the stream | ||
| debug_log("bufferCall: rewinding stream\n\r"); | ||
| ((MultiBufferStream *)inputStream.get())->rewind(); |
There was a problem hiding this comment.
this was a cheat, present to allow for looping without clobbering the call stack in the absence of a "jump" call
since the API now includes "jump" operators, it's been removed, so a "call" of the current buffer will now be done properly, starting up a new processor, and once processing of that buffer is complete execution will continue from where the call had been made as it should do
| if (id != 65535 && inputStream->available() == 0) { | ||
| // tail-call optimise - turn the call into a jump | ||
| bufferJump(bufferId, offset); | ||
| return; | ||
| } |
There was a problem hiding this comment.
to mitigate the potential issue of the call stack getting clobbered, tail-call optimisation is included. basically if we're at the end of a buffer we'll perform a jump instead of a call, as there's no more commands to return to in this buffer.
the id != 65535 check here ensures we're not in the top-level stream processor (connected to the z80), as that's a real comms channel and so inherently can't "jump".
| if (rw == 0 && rh == 0) { | ||
| // TODO support defining bitmap from screen area | ||
| // as per Acorn GXR |
There was a problem hiding this comment.
on the Acorn GXR you could define a bitmap by selecting a screen area with "move" commands and then calling VDU 23,27,1,0,0,0,0,0,0,0,0 (I might not have exactly the right number of zeros there 😁
now that we support more formats of bitmap, it is potentially possible for us to support this behaviour. it's not implemented for now as it will need some experimentation to work out exactly how it operates and all of the potential quirks
| // Extended bitmap commands | ||
| case 0x20: { // Select bitmap, 16-bit buffer ID |
There was a problem hiding this comment.
our "extended" commands here essentially correspond to the original API but with 0x20 added. we only need 3 new commands for our buffer-backed bitmaps
| case 0x21: { // Create bitmap from buffer | ||
| auto width = readWord_t(); if (width == -1) return; | ||
| auto height = readWord_t(); if (height == -1) return; | ||
| auto format = readByte_t(); if (format == -1) return; |
There was a problem hiding this comment.
rather than a stream of bytes at this point, this command is essentially just telling the bitmap system the format of data that is already stored in a buffer, so we just send a single format byte after the width and height
adds support for jump, conditional jump, copy, and consolidate, with placeholders for jump with offset calls
API calls added to bitmap/sprite APIs to allow for the creation of bitmaps based on a buffer also a call added to select bitmap based on buffer ID, allowing us to break the current 256 bitmap limit next step is to integrate this with the existing bitmap system, making all those bitmaps be buffer backed, and to ensure that bitmaps (and sprites) properly handle buffer deletion
all existing bitmap API calls now use buffered commands API buffers for their backing stores existing code should all still work. bitmaps will be assigned buffers in the range &FA00-&FAFF (64000-64255). these buffers are accessible to the buffered commands API and thus are editable still some work to be done ensuring this is safe, mostly in terms of how sprites operate. also bitmaps should really be cleared when their backing buffers are cleared, but that does not currently happen
command added for buffer reverse takes an option value to allow for future expansion. currently only `0` is supported, which will mean the bytes in each stream inside a buffer will be reversed. (the streams stay in-order.)
rather than being ignored, jump to buffer -65535 is now a “jump to end”, which will exit current stream. it is ignored for the ez80 VDU stream also fixed compiler warnings
buffered commands that had offered support for 24-bit offsets now support “advanced” offsets, indicated via the same flag bit when offsets are “advanced” they are still a 24-bit value (as before), but if the top bit of that value is set then the following 16-bit value indicates the block within the buffer that the offset applies to
support for jump with offset added as an added bonus, call with offset added too as it’s needed when jumping from the ez80 VDU processor, so we may as well expose it breaking change - call current buffer no longer rewinds the buffer. this functionality was there because we didn’t have a jump added bonus - tail call optimisation added to bufferCall
reverse command now supports differing value sizes, including arbitrary value sizes, as well as chunking
split can now be used to split into separate buffers, and to split into a given number of chunks by width (allowing for instance a bitmap to be split into columns) spread calls added to spread blocks from a buffer into other buffers split and spread support versions to specific target buffer IDs, or to start from a specified target ID tail-call optimisation fixed (would never have been used as it was - oops) common buffered code refactored for reuse jump to buffer 65535 with an offset is now “jump within buffer”, rather than “jump to end” (which it remains for jumps without an offset)
bitmap and sprite reset calls now also reset the current bitmap/sprite initial/reset bitmap ID is now 64000, i.e the 8-bit bitmap 0. added checks to verify that allocations have happened inside vdu_buffered and vdu_sprites changed bitmap command &21 (create bitmap for buffer) to be more consistent with its corresponding command 1, so it no longer takes a buffer ID and the format moves to the end - both commands thus now work by using a “select bitmap” command first to set the bitmap buffer to use, both accespt width and height as their first 2 parameters, and then comes either the bitmap data stream (for command 1) or the format (for command &21)
and some minor logging enhancements
adds one new audio sample command to “create sample from buffer”, which expects a buffer ID and a format byte the “select waveform” command now understands waveform type of 8 (AUDIO_WAVE_SAMPLE) which then expects the 16-bit buffer ID for the sample. samples added using the existing API will get stored in bufferId &FB00 up sample object changes
allows for samples to be modified in-place, thus you can create a sample for a buffer, and then do things like reverse the blocks in a buffer and hear those changes reflected on sample playback
fixes intended “jump to end” functionality, whereby calling a buffered jump or conditional jump command (the versions without an offset) to buffer 65535 (-1) should “jump to end” (or return) of the current buffer
8208ee9 to
e0f3eae
Compare
make buffer 65535 mean “current buffer” in all places where that makes sense, and disable its use where it doesn’t
e0f3eae to
136e123
Compare
|
|
||
| #define AUDIO_CHANNELS 3 // Default number of audio channels | ||
| #define MAX_AUDIO_CHANNELS 32 // Maximum number of audio channels | ||
| #define MAX_AUDIO_SAMPLES 128 // Maximum number of audio samples |
There was a problem hiding this comment.
moving to use buffers for samples means we no longer need this
|
|
||
| #include "types.h" | ||
| #include "audio_channel.h" | ||
| #include "buffer_stream.h" |
There was a problem hiding this comment.
this class/struct is now based on top of BufferStream objects, rather than keeping/managing its own data block
it also becomes slightly more functional, moving much of the business of how to manage reading sample data into here, rather than having it inside the EnhancedSamplesGenerator
| @@ -0,0 +1,122 @@ | |||
| #ifndef BUFFERS_H | |||
There was a problem hiding this comment.
raw buffer related things are moved into this separate file. this makes using the buffers object slightly simpler
| uint8_t data; | ||
| uint8_t ihexchecksum,ez80checksum; | ||
|
|
||
| bool done,defaultaddress,ez80checksumerror; |
There was a problem hiding this comment.
unused variable removed to get rid of a compiler warning
adds support for jump, conditional jump, copy, and consolidate, with placeholders for jump with offset calls
plus, bitmaps now use buffers!
oh, and samples too!!
bitmap things
bitmaps uploaded the "old-fashioned way" using
VDU 23, 27, 1get stored in buffers numbered &FA00-&FAFF (64000-64255). they therefore become editable using the buffered commands API. all of the existing bitmap and sprite commands work as before - the nature of the API means that just two of them are restricted to using bitmaps in this range.A few new bitmap commands have been added to allow for full 16-bit buffer IDs to be used for bitmaps, thus allowing many more bitmaps than was previously allowed. to facilitate this the bitmaps/sprites API gains three new operators. these correspond to the existing calls but have &20 added to them. they are as follows:
Extended bitmap select, using a 16-bit (buffer) ID:
This command can be used with any buffer ID, but only a buffer that has been marked as containing a bitmap will be able to be drawn.
As a buffer by itself needs some more information before it can be used as a bitmap, we also get a "create bitmap from buffer" command:
This command essentially operates the same as the existing command 1, but instead of providing a stream of data we just let the system know the format of the data to be found in the buffer that has been selected. If the buffer does not exist, or the buffer contains multiple blocks of data (is not a "singular" buffer), then no bitmap will be created.
By passing the format here we gain the ability to support other formats of bitmap.
format is 0 for RGBA8888, 1 for RGBA2222 and 2 for mono/mask. other values will be interpreted as RGBA8888. the call sanity checks, making sure the buffer has the correct amount of data for the requested width, height and format, and if it doesn't then the call will fail and the bitmap won't be created/assigned.
this brings support for RGBA2222 format (and probably mono bitmaps too, but those look slightly weird in fab-gl as they're "mask" rather than mono - I've not tested them yet).
the other new command is a version of the "add bitmap to sprite" command that supports a 16-bit bitmap/buffer ID:
As noted above, buffers need to be "singular" in order to be used as a bitmap. (this is because the underlying graphics system needs bitmaps stored in contiguous chunks of memory.) if you have uploaded multiple blocks to a buffer ID then you will need to consolidate those blocks into one before you can use it. that can be done using the consolidate command:
sample things
samples uploaded using the pre-existing API will be automatically stored in buffers ranging &FB00-&FB7F. sample number -1 gets stored at &FB00, -2 placed at &FB01, -3 at &FB02 etc.
Much like with bitmaps, to use a buffer as a sample you first need to indicate that the buffer contains a sample, and tell the system what format the sample is, which is done with the following audio system command:
Audio commands usually have their channel (or sample) identifier in the byte immediately after
&85- but for this particular command that byte will be ignored.There are currently two audio formats supported.
0is signed 8-bit (the "native" format, which the pre-existing sample load command supports).1indicates the sample is unsigned 8-bit (often a more common format for sample storage).To select a sample for use on an audio channel using a buffer ID then the "set waveform" command is used specifying the waveform type as
8and then providing the buffer ID, as follows:It should be noted that only buffers that have been indicated to be a sample can be selected in this way.
The existing mechanism to select samples using a negative number will still work. This means the following two commands are equivalent:
Both of these will select the same sample for use on channel 1. NB that for either of these commands to work either the sample has to be uploaded using the "sample load" command, or the buffer loaded and then the "use buffer for sample" command needs to be called to indicate the buffer contains a sample
other things
oh yeah, and the buffered commands API now supports Jump and Conditional Jump. versions of those commands with offsets, and copy, as well as the above consolidate command... plus there's also now commands to reverse data in various ways and split data into different blocks and/or across different buffers. at some point (soon) I'll document all of this properly. 😁
OK - documentation for buffered commands API has now been updated. See #48