Skip to content

merge with master 201909 - #11

Open
livius2 wants to merge 887 commits into
livius2:masterfrom
JAM-Software:master
Open

merge with master 201909#11
livius2 wants to merge 887 commits into
livius2:masterfrom
JAM-Software:master

Conversation

@livius2

@livius2 livius2 commented Sep 2, 2019

Copy link
Copy Markdown
Owner

No description provided.

joachimmarder and others added 30 commits August 2, 2024 19:26
…n the value. of parameter ChildNodesOnly"

This reverts commit 9ace431.
…later deal with the fact that StartNode is nil
…rag(). Removed some unused members fromTVTDragImage. Issue #1254
marder and others added 30 commits July 20, 2026 13:00
… current implementation, it might become invisible, if the text scaling in Windows is activated, while the overall scaling for the screen isn't
…tscaling

fix: Adjusted height calculation for the text-editing-caret. With the…
ToggleSelection() - the Shift+Arrow path - removes nodes via
InternalRemoveFromSelection(). That routine only *marks* the entry in FSelection
(it sets the low bit of the pointer, which is what PackArray later looks for) but
fires DoRemoveFromSelection() and Change() straight away. FSelectionCount is not
corrected until PackArray runs after the loop, so every handler invoked in
between sees a count that is too high by the number of nodes already dropped.
Iterating SelectedNodes gives the right answer at the same moment because
vsSelected has been cleared, which is exactly the discrepancy reported.

FSelectionCount cannot simply be decremented when marking: it is also the length
PackArray scans, so lowering it early would leave marked entries in the array.
Instead the pending marks are counted and subtracted in GetSelectedCount, and
SelectedCount now reads that getter instead of the raw field. Internal callers
keep using FSelectionCount directly, so the physical bookkeeping is unchanged.

Event order is deliberately left alone - Change() is called from
InternalRemoveFromSelection() on purpose, see the comment referring to #1047.

The five line pack-and-resize block that appeared at seven call sites is now
PackSelection(), which also resets the pending counter. Centralising it is what
keeps that counter from drifting, since resetting it at seven places is easy to
forget. It returns whether the array was shortened, which is what InvertSelection
used its local flag for.

Adds Tests/VTSelectedCountIssue1197Tests.pas: one test asserts the count seen
during OnRemoveFromSelection, a second asserts the count after the operation so a
future change cannot over-correct. Verified both ways - with the fix the suite is
136 passed / 2 failed, reverting only the getter change puts it back to 135 / 3.
The two remaining failures are the pre-existing TestCopyHTML1 and TestCopyHTML2.

Co-Authored-By: Claude Opus 5 <[email protected]>
…_PRINT

The header lives in the non-client area, and two separate mistakes made rendering
the tree into a caller supplied device context unusable.

WMPaint always fetched a window DC via GetDCEx to draw the header, ignoring the DC
that comes with the message. TWinControl.PaintTo performs WM_PAINT with the target
DC, so the client area ended up in the copy while the header was painted onto the
real window instead - the copy had no header at all. It now paints into Message.DC
when one is supplied.

That alone put the header two pixels off: PaintTo draws the border itself and then
moves the origin inside it, while FHeaderRect is relative to the outer window
corner, so the header was shifted by the border width and clipped on the opposite
edge. csPaintCopy marks exactly that case, and GetBorderDimensions returns negative
values, so adding them shifts back.

WMPrint drew the header regardless of the PRF_ flags, so a PRF_CLIENT only request
got the non-client header painted over the client area, which is the corrupted
border the reporter saw. It now checks PRF_NONCLIENT.

Measured on Delphi 13.1 / Win32 by rendering into an off-screen bitmap and counting
the pixels of a distinctively coloured header, so the result does not depend on
window visibility or theming:

                                        before          after
  PaintTo                               0 px            304 px, (362,2)-(377,20)
  WM_PRINT, PRF_CLIENT only             38 px           0 px
  WM_PRINT, PRF_CLIENT or PRF_NONCLIENT 304 px          304 px, unchanged
  WM_PRINT, PRF_NONCLIENT               304 px          304 px, unchanged

PaintTo is now pixel identical to WM_PRINT with PRF_NONCLIENT.

Tests/VTPaintToIssue632Tests.pas covers all four cases. Three of them fail without
this change and one passes either way, guarding against over-correcting the case
that already worked. Suite goes from 135 passed / 5 failed to 138 passed / 2
failed; the two remaining failures are the pre-existing TestCopyHTML1 and
TestCopyHTML2, which also fail on an unmodified master here.

Co-Authored-By: Claude Opus 5 <[email protected]>
With the explorer theme active, DetermineLineImageAndSelectLevel()
suppresses the tree line under the expand button by overwriting the
last line image entry with ltNone. The band conversion in
PaintTreeLines() however relies on the documented invariant that
ltNone never appears as the last entry: on ltNone it takes over the
style of the entry to the right, which for the last entry is an
out-of-bounds read of the dynamic array (raises ERangeError with
range checking on). The resulting garbage styles made the bands of
every collapsed node with children disappear, while expanded nodes
stayed intact - matching the screenshot in the issue.

Skip the suppression in band mode: bands are box edges, not lines
pointing at a button, so there is nothing to suppress.

Measured offscreen via pixel counting (see new regression test):
identical band rendering with and without the explorer theme state
after the fix; before, 53 of 1954 band pixels were missing in the
test scenario. Test suite: 2 pre-existing failures (TestCopyHTML1/2,
also failing on unmodified master), no new failures.

Co-Authored-By: Claude Fable 5 <[email protected]>
…a mapping mode

SetCanvasOrigin() transformed its shift through LPtoDP before passing
it to SetWindowOrgEx. SetWindowOrgEx however expects logical units -
the same units the tree calculates with - so on a canvas with a
mapping mode (the issue uses MM_ANISOTROPIC with a 2x viewport) the
shift was scaled twice and every node was drawn at twice its offset:
node contents, tree lines, buttons and grid lines all drifted apart,
exactly as shown in the issue. With the default MM_TEXT mapping the
transformation was a no-op, which is why the ordinary paint paths
never showed the problem.

Pass the shift to SetWindowOrgEx untransformed.

Measured offscreen (2x MM_ANISOTROPIC): after the fix the unbuffered
rendering places all grid lines and tree lines pixel-identical to the
buffered rendering, and exactly at twice the unmapped positions; the
MM_TEXT rendering is byte-identical to the one before the fix. New
regression test Tests/VTPaintTreeIssue1074Tests.pas asserts both
properties; without the fix both assertions fail. Test suite: 2
pre-existing failures (TestCopyHTML1/2, also failing on unmodified
master), no new failures.

Co-Authored-By: Claude Fable 5 <[email protected]>
The zip is the reproduction project of a bug report and slipped in
with 85428f2 (Fixed issue #1368).

Co-Authored-By: Claude Fable 5 <[email protected]>
…t, with or without explorer theme

Without the explorer theme the focus rect was drawn around InnerRect
(or CellRect with toGridExtensions) only, although toFullRowSelect
selects and highlights the entire row. Both the draw condition and the
rectangle choice in PrepareCell were gated on tsUseExplorerTheme; in
addition RowRect was only computed when the explorer theme was active.

Compute RowRect unconditionally and drop the two tsUseExplorerTheme
conditions. Each cell draws the row-wide rect clipped to its own
rectangle, so the XOR-based DrawFocusRect touches every pixel exactly
once - measured: the dotted perimeter pixel count matches a single
420x18 rectangle (436 vs. expected 438) with no cancelled segments.

Measured offscreen: without columns the focus rect now spans the whole
client width, with three columns it spans all columns instead of just
the focused one; the explorer-themed rendering is pixel-identical to
before. New regression test Tests/VTFocusRectIssue765Tests.pas asserts
both spans; without the fix both assertions fail. Test suite: 2
pre-existing failures (TestCopyHTML1/2, also failing on unmodified
master), no new failures.

Co-Authored-By: Claude Fable 5 <[email protected]>
…ng during keyboard navigation

Keyboard navigation sets the focused node twice: WMKeyDown moves the
focus (which fires OnFocusChanged), then AddToSelection() assigns the
- by now unchanged - focused node again. SetFocusedNode() always ran
DoFocusNode(), which begins by ending a node edit. An edit started by
the application inside OnFocusChanged was therefore ended again right
away by that redundant assignment. With the mouse the event order
differs, which is why the same handler worked there.

Fix as suggested in the issue discussion: SetFocusedNode() exits early
when the node is already focused, so the property setter has no side
effects for a no-op assignment. DoFocusNode() itself is unchanged, its
deliberate same-node handling (scroll into view) remains available to
its direct callers.

New regression test Tests/VTFocusChangedIssue1379Tests.pas covers the
minimal contract (re-assigning the focused node keeps tsEditing) and
the reported scenario (edit started in OnFocusChanged survives a
VK_DOWN); both fail without the fix. Test suite: 2 pre-existing
failures (TestCopyHTML1/2, also failing on unmodified master), no new
failures.

Co-Authored-By: Claude Fable 5 <[email protected]>
…rea and were trapped there

Dropping a normal column in front of a fixed column made it fixed
(deliberate behavior of TVirtualTreeColumn.SetPosition when a column
enters the fixed area) - and since issue #1314 fixed columns lose
coDraggable, so the column could never be dragged out again.

Redirect the drop target in TVTHeader.DragTo(): when a non-fixed
column is dragged over a fixed one, the target becomes the first
non-fixed visible column instead. Drop mark and drop are consistent
and the column lands right after the fixed area, staying normal and
draggable. The programmatic path is unchanged: assigning Position
directly still moves a column into the fixed area and makes it fixed.

New regression test Tests/VTFixedColumnDragIssue1377Tests.pas drives
DragIndex/DragTo/ColumnDropped directly and asserts the redirected
target, the resulting position and the preserved options; it fails
without the fix. A second test pins the unchanged programmatic
behavior. Test suite: 2 pre-existing failures (TestCopyHTML1/2, also
failing on unmodified master), no new failures.

Co-Authored-By: Claude Fable 5 <[email protected]>
Fix #632: header is missing from PaintTo and drawn for client-only WM_PRINT
…-zip

Remove accidentally committed repro project from the repository root
…drag

Fixed issue #1377: Normal columns could be dropped inside the fixed area and were trapped there
Fixed issue #1091: Bands disappear with explorer theme
Fixed issue #1074: PaintTree with poUnbuffered misplaces nodes under a mapping mode
Fix solid black tree on Windows 7 without DWM composition (Delphi 12), closes #1389
…ount

Fix #1197: SelectedCount is stale while selection change events run
Fixed issue #765: Focus rect covers the whole row with toFullRowSelect, independent of the explorer theme
Fixed issue #1379: Redundant focused-node assignment ended node editing during keyboard navigation
In the classic (non-themed) paint path DrawBackground fills the area right of
the last column with Header.Background, but PaintColumnHeader painted the
column cells via DrawEdge with BF_MIDDLE, which always fills the interior with
clBtnFace - a custom Header.Background only ever showed up in the filler area.
The cells now get filled explicitly with Header.Background before the edges
are drawn. For the default clBtnFace the result is pixel-identical, and the
themed and VCL-styles paths are untouched.

Tests/VTHeaderBackgroundTests.pas renders the header offscreen and checks both
directions: cells follow a custom Header.Background (fails without the fix)
and the default rendering keeps the clBtnFace look (guards the no-change
promise).

Co-Authored-By: Claude Fable 5 <[email protected]>
…assic

Fixed classic header cells ignoring Header.Background
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.