Development/compositorclient multi client rendering#325
Conversation
…ns and timeout handling
…osition client render
| png_structp png = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr, PngErrorFn, PngWarnFn); | ||
| if (!png) { fclose(fp); return result; } | ||
|
|
||
| png_infop info = png_create_info_struct(png); |
There was a problem hiding this comment.
Hi @bramoosterhuis : This code from l.45-l.72 is coming up as a good match to several components, some with nasty licenses. Where did it come from? If it is a straight copy, it may need credit depending on the source.
On the other hand, if you wrote it yourself, you can defend that this is boilerplate usage of libpng. Line 62 could then do with a longer comment so it is clear that you know what you are doing and it has to be this way. You could also refer to http://www.libpng.org/pub/png/libpng-manual.txt before line 45 and say that you are following the instructions therein.
Thank you!
There was a problem hiding this comment.
Hi @mhughesacn,
This code was written independently following the official libpng documentation (http://www.libpng.org/pub/png/libpng-manual.txt, Section III.2), not copied from GPL sources. The transformation sequence is standard boilerplate for normalizing any PNG format to RGBA8.
I've added comprehensive documentation: a reference to libpng-manual.txt before line 45, an expanded comment at line 62 explaining the transformation pipeline, and inline comments for each transformation showing intent and understanding.
The similarity to other projects is expected - this is the standard approach documented in the official libpng manual for format normalization.
Happy to provide additional details if needed.
|
Thank you @bramoosterhuis : I have cleared off the match in blackduck (not in the PR), but my apologies, I missed this last night: where did ml-tv-color-small.png come from? |
No problem @mhughesacn! To be fully transparent: I originally created ml-tv-color-small.png for the Mesa renderer tests in ThunderNanoServices and reused it here. It's a test pattern inspired by the Metrological logo designed to verify color channel rendering. |
|
Perfect - thank you @bramoosterhuis - cleared the downvote in the PR. |
There was a problem hiding this comment.
Hi @bramoosterhuis : Sorry if I missed this last month, but where does this image come from?
There was a problem hiding this comment.
This file was generated by me using the tool at https://evanw.github.io/font-texture-generator/. The generator tool itself is CC0 (public domain): https://creativecommons.org/publicdomain/zero/1.0/. The resulting PNG doesn't appear to be licensed, but if this is an issue I can create my own SDF font atlas in GIMP.
There was a problem hiding this comment.
That's great Bram. I think it is reasonable to include the generated image under our default Apache license as a data file. Thank you!
|
Approved but cannot merge as "Could NOT find PNG (missing: PNG_LIBRARY PNG_PNG_INCLUDE_DIR)" is failing some builds |
Use relative path for ThunderClientLibraries workflow reference Changed from absolute repository reference (@master) to relative path (./.github/workflows/) for the ThunderClientLibraries build template. Problem: The previous @master reference always pulled the workflow from the master branch, even during PR builds. This created a chicken-and-egg situation where workflow changes (like adding build dependencies) couldn't be tested in PRs before merging to master. Solution: Using a relative path automatically references the workflow from the same commit that triggered the build. This means: - PRs test with the PR's version of the workflow template - Pushes to master use master's version of the workflow template - No dynamic expression workarounds needed This allows proper testing of workflow changes (such as the libpng-dev dependency addition) within PRs before merging. Note: Thunder and ThunderInterfaces jobs remain with @master references since they reference external repositories.
|
@VeithMetro FYI: I updated the workflow to use relative paths for the reusable workflow reference. This allows PRs to test with their own version of the build template instead of always using master. Now we can properly test workflow changes (like adding build dependencies) before merging. |
MFransen69
deleted the
development/compositorclient-multi-client-rendering
branch
4 participants
This PR improves compositor client reliability and adds comprehensive testing infrastructure:
Key Changes:
Connection validation -
Compositor::IDisplay::Instance()now returnsnullptrwhen the remote compositor connection fails, allowing applications to handle connection failures gracefully instead of crashing.Test infrastructure - Added complete client rendering test suite including:
Buffer lifecycle management - Improved buffer acquisition/release semantics with proper state tracking. The client now correctly signals when new frames are presented vs when the same frame is being re-composited.
Enhanced error handling - Replaced
fprintfdebugging with proper TRACE macros for consistent logging. Added dedicatedBufferInfoandBufferErrortrace categories for fine-grained buffer operation debugging.GBM surface integration - Proper handling of
gbm_surface_lock_front_buffer()timing and orphaned buffer cleanup. Added detailed trace logging for buffer pool management.Testing Features:
This PR works in conjunction with ThunderNanoServices PR rdkcentral/ThunderNanoServices#949 to resolve compositor synchronization issues.