Issue 318 in cocotron: Variable row height support for NSTableView.
codesite...@google.com
Comment #3 on issue 318 by m...@jump-ing.de: Variable row height support
for NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
Now I think I'm done.
New with this patch:
- The delegate is asked for the actual height of each row. These values are
cached, as they are used pretty
often.
- NSTableView.h has now a few notes on how to subclass NSTableView without
conflicting with the cache
mechanism.
- -noteHeightOfRowsWithIndexesChanged: is implemented.
- NSOutlineView is made aware of the new instance variable name (should use
[self rowHeight], but that's a
different topic).
- Fixed drawing of the alternating background for unused tabe view /
outline view space. Remaining space is
drawn with the standard row height (+ intercell spacing), as set per
default to 16.0 pixel or by -rowHeight:.
- Variable height rows for outline views might work, I haven't tested this.
- Fixed a bug in -[NSOutlineView mouseDown:] where clicking beyond the last
column or beyond the last row
resulted in an exception.
The attached implementation works fine here and is verified against Apple's
documentation. Please apply,
please test.
Attachments:
NSTableView-variable-row-height-patch.txt 27.7 KB
--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings
codesite...@google.com
Labels: -Type-Defect Type-Enhancement
Comment #4 on issue 318 by rolf.anroni: Variable row height support for
NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
I gave the changes a try within in my application (using Cocotron r528),
and with the patch applied there are
issues with the horizontal grid lines. They are missing below empty table
rows, and one horizontal grid line is
missing after the first row. In addition they are off by on pixel to the
top, making my cell contents looking
misaligned. Finally also the vertical grid lines are offseted by 1 to the
left.
I attached two screenshots of a table from my application before and after
the patch was applied. For the
reference I attached also a screenshot of the same table drawn at Mac OS X.
Sorry, but I did not come to test the variable row hight enhancements. For
me this addition would be OK even
untested, if it would not introduce regressions in drawing of tables with
fixed row height.
Attachments:
mac.png 93.1 KB
codesite...@google.com
Comment #5 on issue 318 by rolf.anroni: Variable row height support for
NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
Sorry, I made a mistake when attaching the files. Here come the missing
ones.
Attachments:
before.png 20.3 KB
after.png 19.6 KB
codesite...@google.com
Comment #6 on issue 318 by m...@jump-ing.de: Variable row height support
for NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
First of all: thanks for giving it a try. The pictures were helpful.
Regarding the grid of the first row not drawn:
There was a leftover from debug code. Removed.
Regarding grid not drawn beyond the last row:
Added code to do so.
Regarding "one pixel off":
This is true and this is intentional. While the current Cocotron draws the
topmost row of pixels for each table
row, Cocoa draws the bottom most one. The patch favours the Cocoa method
and I verified accuracy by
overlaying screenshots.
Other issues fixed:
- The bit mask for grid drawing was declared wrongly in NSTableView.h.
Changed this to match Cocoa's.
- Removed the obsolete and unused instance variable _drawsGrid.
- NSOutlineView did set drawing vertical and horizontal grid lines to YES
in -initWithCoder unconditionally.
Removed this to keep what's in the NIB.
- Nevertheless, NSOutlineView forgot to invoke super in
-drawGridInClipRect:. Added this to get the grid
drawn.
Well, I hope I addressed all the issues now.
Attachments:
NSTableView-variable-row-height-refined-patch.txt 29.8 KB
codesite...@google.com
Comment #7 on issue 318 by m...@jump-ing.de: Variable row height support
for NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
BTW, the patch is against SVN r531 now (and I'd love to use Git instead of
SVN for the ability to keep local
patchsets).
codesite...@google.com
Comment #8 on issue 318 by rolf.anroni: Variable row height support for
NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
Regarding "one pixel off". I will be OK with this, I only will need to
rework the vertical and horizontal position
adjustments in -[NSTableView _adjustedFrame:forCell:].
BTW: I discovered some problems with table redrawing on certain actions,
that were introduced by your
patches that replaced some calls [self setNeedsDisplay:YES] by [self
setNeedsDisplayInRect:SOMEROWRECT]. I
am working in the moment on focus ring behaviour in NSCell itself, but in
the moment the focus ring in table
views is emulated by drawing a gray border, and this becomes not deleted
completely when redrawing only
and strictly the row rect. In addition, editing a cell did not end, when
clicking into the empty table region.
In my local version of NSTableView, both of these problems are resolved,
and I also fixed a memory leak.
I would like to ask you for a little patience about the focus ring
emulation. This will of course be removed as
soon as I have finished the focus ring work in NSCell and its most
important sub classes, which may take
some time. For the time being it would be nice if tables could be
nonetheless look nice.
Unfortunately at the upcoming weekend, I won't have access to a Win-Box,
and I cannot test anything until
monday. I am also in a hurry now, and I do not want to submit my changes to
NSTableView in that state that
might interfere with your patch. So please, pardon me, that I attach to
this comment my current local version
of NSTableView.m, perhaps you might want to try to apply your patch on this.
I am sure that we can conclude on this at latest at monday.
Happy weekend and best regards
Rolf
Attachments:
NSTableView.m 53.3 KB
codesite...@google.com
Owner: rolf.anroni
Comment #9 on issue 318 by rolf.anroni: Variable row height support for
NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
Markus, I am right now reviewing and evaluating your patch for the purpose
of committing it.
I just ran across a first doubt:
In comment #6 on this issue, you wrote:
- The bit mask for grid drawing was declared wrongly in NSTableView.h.
Changed this to match Cocoa's.
Looking at the patch, my understanding is, that your "fix" lexically
matches the declaration to that of Cocoa,
while from a functional perspective, the original declaration was OK,
wasn't it?
Best regards
Rolf
codesite...@google.com
Status: Fixed
Comment #10 on issue 318 by rolf.anroni: Variable row height support for
NSTableView.
http://code.google.com/p/cocotron/issues/detail?id=318
(No comment was entered for this change.)
Markus Hitter
Am 06.07.2009 um 19:29 schrieb codesite...@google.com:
> Looking at the patch, my understanding is, that your "fix"
> lexically matches the declaration to that of Cocoa, while from a
> functional perspective, the original declaration was OK, wasn't it?
The original didn't define a bitmask, but a continuous integer
without specifying any of the values. Now, gcc's default lowest value
is 0, the default increment is 1, decimal 2 happens to be represented
by a binary 0010 and all bits higher than the lowest two are
currently unused.
For all this together you're right, the original declaraction gave
identical results. Feel free to change it back to the continuous
integer.
Markus
Rolf Jansen
cases where equivalent coding and formatting is possible, we need to
avoid even pieces of lexical identity to Cocoa headers. In the present
case we had a perfect functional alternative which does not rise
copyright issues. It is for this reason, that I did exclude this
change when committing the rest of your patch. It is definitely not,
that I disliked the coding style. If you like, I can add a comment,
that this enum is a bitmask declaration, and if ever it will be
enhanced, the next values should be 4, 8, 16, ...
Best regards
Rolf
Markus Hitter
Am 07.07.2009 um 00:52 schrieb Rolf Jansen:
> My understanding from previous discussions with Chris is, that in
> cases where equivalent coding and formatting is possible, we need to
> avoid even pieces of lexical identity to Cocoa headers.
OK, that's news to me. So far I assumed Chris prefers lexical
identity as he prefers identical instance variable names as well
(which I probably yet again missed to follow *sigh*).
> If you like, I can add a comment,
> that this enum is a bitmask declaration, and if ever it will be
> enhanced, the next values should be 4, 8, 16, ...
Yes, a comment whould likely reduce confusion. Another often used way
to declare bitmaps is to use explicit hex values:
enum {
NSTableViewGridNone = 0x00000000,
NSTableViewSolidVerticalGridLineMask = 0x00000001,
NSTableViewSolidHorizontalGridLineMask = 0x00000002
};
Nothing to make a headache about, though. Anyways, thanks for
reviewing and commiting.
Markus