Change the WinIMergeLib splitter position API to use splitter ratios instead of absolute positions. - #40
Conversation
…instead of absolute positions.
There was a problem hiding this comment.
🟡 Changes recommended
The current ratio implementation has correctness/API issues (notably ratio initialization and pre-window persistence behaviors) that can lead to invalid layouts or prevent reliably restoring saved splitter positions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates WinIMergeLib’s image-merge window splitter API to persist splitter layout as ratios (rather than absolute pixel positions), enabling splitter positions to be restored consistently across window resizes and supporting multi-pane comparisons.
Changes:
- Replaces
GetSplitterPosition()/SetSplitterPosition()withGetSplitterRatio(),SetSplitterRatios(), andResetSplitterRatios(). - Introduces internal storage for splitter ratios and applies them on resize via
ApplySplitterRatio(). - Updates ratio state when the user drags a splitter via
UpdateSplitterRatios().
File summaries
| File | Description |
|---|---|
src/WinIMergeLib/WinIMergeLib.h |
Updates the public IImgMergeWindow splitter API to expose ratio-based getters/setters and reset. |
src/WinIMergeLib/ImgMergeWindow.hpp |
Implements ratio storage/application, initializes default ratios, applies ratios on resize, and updates ratios on splitter drag. |
Review details
Suppressed comments (2)
src/WinIMergeLib/ImgMergeWindow.hpp:934
- OpenImages has the same ratio-initialization issue as NewImages: only checking m_splitterRatios[0] can leave ratio[1] at -1 when changing from 2 to 3 images, which can produce invalid pane sizes when applying ratios.
if (m_splitterRatios[0] < 0.0)
{
for (int i = 0; i < nImages - 1; ++i)
m_splitterRatios[i] = 1.0 / nImages;
}
src/WinIMergeLib/ImgMergeWindow.hpp:1503
- UpdateSplitterRatios repeats GetWindowRect() calls in the horizontal-split loop as well; caching the RECT once per pane avoids redundant work.
for (int i = 0; i < m_nImages - 1; ++i)
{
int paneHeight = m_imgWindow[i].GetWindowRect().bottom - m_imgWindow[i].GetWindowRect().top;
m_splitterRatios[i] = static_cast<double>(paneHeight) / totalHeight;
}
- Files reviewed: 2/2 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…instead of absolute positions.
…instead of absolute positions.
…instead of absolute positions.
…instead of absolute positions.
…instead of absolute positions.
Summary
Change the WinIMergeLib splitter position API to use splitter ratios instead of absolute positions.
Changes
Replace
GetSplitterPosition()/SetSplitterPosition()with:GetSplitterRatio()SetSplitterRatios()ResetSplitterRatios()Support multiple splitter ratios for multi-pane image comparison.
Store splitter positions as ratios so that they can be restored correctly when the window size changes.
Apply the stored splitter ratios when the image comparison window is resized.
Update splitter ratios when the splitter is moved.
This change is required to support remembering splitter positions in the WinMerge image comparison window.