[EGD-7255] Fix vector image rendering

[GravisZro]

Description Fixes vector image rendering by that wouldn't read the full number of rows nor the pixel offset. The image validation check was moved outside of the loops as well.

Your checklist for this pull request

Make sure that this PR:

• [x] Complies with our guidelines for contributions
• [x] Has unit tests if possible.
• [x] Has documentation updated

[SP2FET]

Hi, sorry for the long delay with your PR review. Could you please add some unit tests to be able to confirm that your code is working correctly?

[GravisZro]

Hi, sorry for the long delay with your PR review. Could you please add some unit tests to be able to confirm that your code is working correctly?

From what I can tell, there are no existing unit tests for verifying anything in module-gui/gui/core or any type of rendering whatsoever. This leads me to think that what you are asking for is substantial enough to require it's own PR. It's not that I'm not capable but it seems a bit much to ask some internet rando volunteer who just wanted to submit a simple bugfix.

[SP2FET]

Hi, sorry for the long delay with your PR review. Could you please add some unit tests to be able to confirm that your code is working correctly?

From what I can tell, there are no existing unit tests for verifying anything in module-gui/gui/core or any type of rendering whatsoever. This leads me to think that what you are asking for is substantial enough to require it's own PR. It's not that I'm not capable but it seems a bit much to ask some internet rando volunteer who just wanted to submit a simple bugfix.

Sure - I've created the unit test for the vector rendering as this is a pretty crucial part of the code and it should be covered with tests - if it's not, the modification of it is a good cause do write them :) https://github.com/mudita/MuditaOS/pull/3548

[SP2FET]

as those tests are merged, please rebase your branch - if it will pass all the check, we'll merge it ;)

[SP2FET]

Please format the code using tools mentioned in the docs, as wrong style is blocking the checks

[SP2FET]

to do the style fix please execute: ./config/style_check_hook.sh --branch-fix

[GravisZro]

to do the style fix please execute: ./config/style_check_hook.sh --branch-fix

Thank you for the tip because I'm still having problems bootstrapping the build system again.

[SP2FET]

Hi, unfortunately, the unit tests have failed. Let me show you the output from the CI:

test 170
        Start 170: Draw vector image test

170: Test command: /home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui "Draw vector image test"
170: Test timeout computed to be: 10000000
170: =================================================================
170: ==4791==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6190000058af at pc 0x7fbe5a21cf26 bp 0x7fff4d06b470 sp 0x7fff4d06ac18
170: WRITE of size 1 at 0x6190000058af thread T0
170:     #0 0x7fbe5a21cf25 in __interceptor_memset ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:795
170:     #1 0x55e8462afc50 in gui::DrawImage::drawVecMap(gui::Context*, gui::VecMap*) const /home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/module-gui/gui/core/DrawCommand.cpp:247
170:     #2 0x55e8462afffe in gui::DrawImage::draw(gui::Context*) const /home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/module-gui/gui/core/DrawCommand.cpp:278
170:     #3 0x55e846277b5d in ____C_A_T_C_H____T_E_S_T____9() (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0x249b5d)
170:     #4 0x55e846106434 in Catch::TestInvokerAsFunction::invoke() const (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xd8434)
170:     #5 0x55e846103ffd in Catch::TestCase::invoke() const (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xd5ffd)
170:     #6 0x55e8460f4068 in Catch::RunContext::invokeActiveTestCase() (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xc6068)
170:     #7 0x55e8460f387b in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xc587b)
170:     #8 0x55e8460effe1 in Catch::RunContext::runTest(Catch::TestCase const&) (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xc1fe1)
170:     #9 0x55e8460f8755 in Catch::(anonymous namespace)::TestGroup::execute() (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xca755)
170:     #10 0x55e8460fcb9d in Catch::Session::runInternal() (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xceb9d)
170:     #11 0x55e8460fc41f in Catch::Session::run() (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0xce41f)
170:     #12 0x55e846160f5d in int Catch::Session::run<char>(int, char const* const*) (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0x132f5d)
170:     #13 0x55e8461357ae in main (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0x1077ae)
170:     #14 0x7fbe59c3e0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
170:     #15 0x55e8460b130d in _start (/home/jenkins/workspace/MuditaOS_PR_Builder__Community/MuditaOS/build-PurePhone-linux-Debug/catch2-gui+0x8330d)
170: 
170: Address 0x6190000058af is a wild pointer.
170: SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:795 in __interceptor_memset
170: Shadow bytes around the buggy address:
170:   0x0c327fff8ac0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170: =>0x0c327fff8b10: fa fa fa fa fa[fa]fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8b20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8b30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8b40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8b50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170:   0x0c327fff8b60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
170: Shadow byte legend (one shadow byte represents 8 application bytes):
170:   Addressable:           00
170:   Partially addressable: 01 02 03 04 05 06 07 
170:   Heap left redzone:       fa
170:   Freed heap region:       fd
170:   Stack left redzone:      f1
170:   Stack mid redzone:       f2
170:   Stack right redzone:     f3
170:   Stack after return:      f5
170:   Stack use after scope:   f8
170:   Global redzone:          f9
170:   Global init order:       f6
170:   Poisoned by user:        f7
170:   Container overflow:      fc
170:   Array cookie:            ac
170:   Intra object redzone:    bb
170:   ASan internal:           fe
170:   Left alloca redzone:     ca
170:   Right alloca redzone:    cb
170:   Shadow gap:              cc
170: ==4791==ABORTING
170/526 Test #170: Draw vector image test ..............................................................................***Failed    0.25 sec

You can also execute those unit test by hand, compiling the check target in the linux build

[CLAassistant]

All committers have signed the CLA.

[GravisZro]

@SP2FET The code is fixed (it was an endianness issue) and it passes the unit test but in order to add a unit test to test large images, I will need a vpi image file to test it with (e.g. 300px X 270px). I don't think the image conversion tools is posted but if it is then tell me. I could write my own but it would introduce the possibility of being a malformed vpi image file.

[SP2FET]

As @paweljonskim has better knowledge of this section of code, he will answer your questions.

[GravisZro]

@SP2FET I've made an additional unit test for large VPI images. I believe this PR is ready for merging.

[Lefucjusz]

@GravisZro please do a rebase and run style checking on your code, so our checks can pass and we'll be able to merge your changes.

[GravisZro]

@GravisZro please do a rebase and run style checking on your code, so our checks can pass and we'll be able to merge your changes.

Everything should be up to date now.