Task 16 implementation. #63
Conversation
Co-authored-by: Erik Bock <[email protected]>
| #include <FramebufferAccess.h> | ||
| #include "SMCE_dll.hpp" |
There was a problem hiding this comment.
ideally these shoudnt go in the public header to avoid leaking stuff we dont want the user to stumble upon.
you can forward declare FramebufferAccess in the header and then store it as a pointer or unique_ptr. this way the definition is hidden
There was a problem hiding this comment.
(And the class name should be prefixed with SMCE__ so as to not conflict with user-defined identifiers)
| class RGBMatrixClass : public ArduinoGraphics { | ||
| private: | ||
| FramebufferAccess framebufferAccessor; | ||
| std::byte buf[(BITS_PER_PIXEL * RGB_MATRIX_HEIGHT * RGB_MATRIX_WIDTH) / CHAR_BIT]; |
There was a problem hiding this comment.
std::byte is relatively new, use uint8_t instead to be more compatible
| func visualize_content() -> Array: | ||
| var text = " Resolution: %dx%d" % [resolution.x, resolution.y] | ||
| return [texture, aspect_ratio, text] |
There was a problem hiding this comment.
Since ScreenBufferVisualizer is very specific this method is not needed and can be put inside the visualizer class
There was a problem hiding this comment.
Should we put setters in ScreenBufferVisualizer and set the texture, aspect_ratio, and text value from the ScreenBuffer's _physics_process() method instead?
There was a problem hiding this comment.
Doesn't ScreenBufferVisualizer already take self ? it can just directly access the fields here
| var viewport: Viewport = Viewport.new() | ||
| var viewport_container: ViewportContainer = ViewportContainer.new() |
There was a problem hiding this comment.
Why are we using a viewport here?
| export var display_height = 0.2 | ||
| var resolution = Vector2(0, 0) # Base resolution (for MKRRGBMAtrix) | ||
| var image_buf = null | ||
| var fps = 1 |
| // | ||
| // Created by danie on 2021-11-02. | ||
| // | ||
|
|
| #define RGB_MATRIX_WIDTH 640 | ||
| #define RGB_MATRIX_HEIGHT 480 | ||
| #define PIXEL_FORMAT SMCE_Pixel_Format::RGB888 | ||
| #define BITS_PER_PIXEL 24 | ||
| #define FPS 60 |
There was a problem hiding this comment.
I doubt all of these are part of the MKRRGB API. If not, they should either be deleted, or prefixed with SMCE__
| "library_patches" / "smartcar_shield") | ||
| .generic_string(), | ||
| .defaults = smce::PluginManifest::Defaults::arduino}}}}; | ||
| std::vector<std::string> dep = {"ArduinoGraphics"}; |
There was a problem hiding this comment.
What has this vector done to you for you to copy it it and thus exclude it from being directly initialized where it's used?
There was a problem hiding this comment.
If I remember correctly, we got a "C1001: Internal compiler error" when using .depends = {"ArduinoGraphics"} directly, and this was just a quick fix for that.
Implementation of task 16 by group 1 (Erik and Daniel).
Description: Add to libSMCE the ability to draw on an output screenbuffer, and add to the frontend a display screen component.