Skip to content

Conversation

TerryFogg
Copy link
Contributor

@TerryFogg TerryFogg commented Nov 6, 2023

Description

The code has been modified to only use ARGB or RGR565 format.

Code was also modified to the RotateImage to properly rotate images with a transparent background.
This was not in place, although it was already in place for stretch image.

Fixed a bug.
In testing on the STM32F769I_Discovery I ran into an exception.
Native code working on an ESP32WroverKitV4.1 would create an exception in the Bitmap_Decoder.cpp.
I eventually tracked this down to a
ARM: STM32F7: hard fault caused by unaligned Memory Access
Included in this code is the fix for this problem as recommended by keil.

Motivation and Context

Problems with colours. This eliminates some confusion with the formats in memory.

How Has This Been Tested?

The modifed code was tested on

  1. STM32769I_Discovery board
  2. ESP32 Wrover Kit v4.1
  3. ESP32 Wrover Kit 4.1 with Generic SPI class and C# class of ili9341 from the Managed Drivers.

All three tests showed the same resulting colours.
See the following testing notes below

Testing notes

  • A selection of 9 ARGB colours was selected to test operation

  • 16-bit RGB565 values expected to be in the native bitmap for presentation to the display controller to properly send to the display device. Examination of the memory buffer of the Bitmap showed that it was correctly in the RGB565 format stored as little endian.

  • RGB565 format is commonly called "native" format in the C++/C code

  • JPEG test was created using Paint.net to set the 9 pixels to the ARGB colours then saved to disk and re-read. The values re-read from the disk are "compressed" so are slightly different. The expected values were found using the eye-dropper tool of paint.net to get the hex colour values.

  • Bitmaps saved in the resource file are all converted to RGB565 format before being embedded into the assembly and deployed to flash. ( This is by design)

  • The graphics code supports reading in bitmaps of 32-bit, 24-bit and 8-bit indexed, usually from file. For this test the bitmaps are setup as a C# byte array to eliminate the need to have a file system.

  • Restoring RGB565 to ARGB

    Restored results are inexact values due to the loss of 3 bits red, 2 bits green and 3 bits blue. The restoration code uses an algorithm originally from .netMF to approximate the original ARGB colour.

  • Gif values should be the same as SetPixel, GetPixel, unlike jpeg GIF uses lossless compression.

// |-------------------|---------------|----------------|---------------|--------------| 
// |     Colours       | 32-bit ARGB   |   Set Pixel    |  Get Pixel    |      JPEG    |
// |                   |  Source       | 16-bit RGB565  | Restored ARGB | Restored RGB |
// |-------------------|---------------|----------------|---------------|--------------|
// | Color.DarkRed     |  0xFF8B0000   |    0x8800      |  0xFF8F0000   |   0x8F0000   |
// | Color.Red         |  0xFFFF0000   |    0xF800      |  0xFFFF0000   |   0xFE0000   |
// | Color.LightPink   |  0xFFFFB6C1   |    0xFDB8      |  0xFFFFB7C0   |   0xFFB6C1   |
// | Color.LightGreen  |  0xFF90EE90   |    0x9772      |  0xFF90EF90   |   0x90EE90   |
// | Color.Green       |  0xFF008000   |    0x0400      |  0xFF008000   |   0x008001   |
// | Color.DarkGreen   |  0xFF006400   |    0x0320      |  0xFF006700   |   0x006401   |
// | Color.LightBlue   |  0xFFADD8E6   |    0xAEDC      |  0xFFAFD8E0   |   0xAFD9E6   |
// | Color.Blue        |  0xFF0000FF   |    0x001F      |  0xFF0000FF   |   0x0000FE   |
// | Color.DarkBlue);  |  0xFF00008B   |    0x0011      |  0xFF00008F   |   0x01008C   |
// |-------------------|---------------|----------------|---------------|--------------|

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

@nfbot nfbot changed the title All graphics methods now use 32-bit ARGB or RGB565 format, BGR format has been removed. All graphics methods now use 32-bit ARGB or RGB565 format, BGR format has been removed Nov 6, 2023
@josesimoes josesimoes changed the title All graphics methods now use 32-bit ARGB or RGB565 format, BGR format has been removed All graphics methods now use 32-bit ARGB or RGB565 format instead of BGR Nov 6, 2023
@josesimoes josesimoes added the Area: Common libs Everything related with common libraries label Nov 6, 2023
nfbot and others added 2 commits November 6, 2023 09:49
Automated fixes for code style.
…b7c-2ccc-474b-820f-1b8c2e03cdd7

Code style fixes for nanoframework/nf-interpreter PR#2811
// Disable the MPU
// ---------------

__DMB(); // Make sure outstanding transfers are done
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per project code style: please move comment to line above, not inlined.

MPU->CTRL = 0; // Disable the MPU and clear the control register

// ---------------
// Configure the region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK to use just a single comment line. No need for the "boxed" version 😉

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TerryFogg thank you for taking care of this!
Can you please extract the memory access fix to a separate PR?
We want PRs to be atomic and this one is about RGB fixes, memory access is unrelated.
Plus having it separated, make is clear about the issue & fix and its easily discoverable in the code history in case it's needed in the future.

@josesimoes
Copy link
Member

@TerryFogg I've fixed the code formatting of targets/ChibiOS/ST_STM32F769I_DISCOVERY/target_external_memory.c so that the PR with the required changes are clear and not masked by the code style changes.

@TerryFogg TerryFogg closed this by deleting the head repository Nov 7, 2023
@nfbot nfbot added invalid and removed Type: bug Type: enhancement Area: Common libs Everything related with common libraries labels Nov 7, 2023
@TerryFogg
Copy link
Contributor Author

I have submitted a separate PR #2820 for the STM32F769 series chip unaligned memory problem.
Once you have put that through, I will run my graphics test for re-verification, before re-submitting the graphics changes without the unaligned memory access

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants