Skip to content

add PersistLoadWatermark #482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MFransen69 merged 2 commits into from
Feb 6, 2026
Merged

add PersistLoadWatermark #482

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96c2d79
add PersistLoadWatermark
MarkTowers9 Jun 13, 2023
5d60fa9
Merge branch 'master' into RDK-42408
MFransen69 Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions interfaces/IWatermark.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace Exchange {
virtual bool ShowWatermark(const bool show) = 0;
virtual bool CreateWatermark(uint32_t id, uint32_t zorder) = 0;
virtual bool UpdateWatermark(uint32_t id, uint32_t key, uint32_t size) = 0;
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The method lacks documentation explaining its purpose, parameters, and return value semantics. Since this is an interface method being added to support persistent watermark features, it should include documentation that explains: what "persist and load" means in this context, what the 'id' parameter represents, and what the boolean return value indicates (success/failure conditions). This is especially important for interface methods that will be implemented by external parties.

Suggested change
virtual bool UpdateWatermark(uint32_t id, uint32_t key, uint32_t size) = 0;
virtual bool UpdateWatermark(uint32_t id, uint32_t key, uint32_t size) = 0;
/**
* Persist and/or load the state of the watermark identified by the given ID.
*
* Implementations typically use this method to store the current configuration
* of a watermark to persistent storage and/or to restore a previously stored
* configuration for the same watermark. The exact persist/load behavior may
* depend on the concrete implementation.
*
* @param id
* The unique identifier of the watermark whose state should be persisted
* and/or loaded.
*
* @return
* true if the persist/load operation completed successfully; false if the
* operation failed (for example due to invalid ID, missing persisted data,
* or storage errors).
*/

Copilot uses AI. Check for mistakes.
virtual bool PersistLoadWatermark(uint32_t id) = 0;
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The method name "PersistLoadWatermark" combines two distinct operations (persist and load) which violates the single responsibility principle. This creates ambiguity about what the method does. Consider splitting this into two separate methods: "PersistWatermark" for saving and "LoadWatermark" for loading, or clarify the intended behavior with a more descriptive name like "LoadPersistedWatermark" if it only loads a previously persisted watermark.

Suggested change
virtual bool PersistLoadWatermark(uint32_t id) = 0;
virtual bool LoadPersistedWatermark(uint32_t id) = 0;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Adding a new method to an existing interface is a breaking change for the API. This change will break binary compatibility because any existing implementations of the IWatermark interface will not have this method implemented, causing vtable mismatches. Consider versioning the interface (e.g., IWatermark2) or using a COM versioning strategy to maintain backward compatibility with existing implementations.

Copilot uses AI. Check for mistakes.
virtual bool AdjustWatermark(uint32_t id, uint32_t zorder) = 0;
virtual bool DeleteWatermark(uint32_t id) = 0;
virtual PalettedImageData GetPalettedWatermark(uint32_t id) = 0;
Expand Down
Loading