TUISlider #151

avaidyam · 2012-12-20T05:07:43Z

Basically a simple slider for TwUI. Drawing is abstracted into overridable methods, and it uses NSSliderCell to draw itself, while still remaining customizable. This allows the slider to be drawn by CoreUI, and remain OS dependent in look and feel.

jwilling · 2012-12-20T15:13:41Z

Hmm, the tracking isn't correct on this. When the slider is near the edges it tracks from the respective edge instead of the center of the knob.

avaidyam · 2012-12-21T02:10:44Z

@jwilling Just noticed that, and I'll fix that soon. I forgot to even send actions when the value changed... oops!

CodaFi · 2012-12-28T00:13:05Z

lib/UIKit/TUISlider.h

+
+// The number of the slider's tick marks. The tick marks assigned to
+// the minimum and maximum values are included.
+@property (nonatomic, assign) NSInteger numberOfTickMarks;


Any reason why this isn't unsigned?

@CodaFi Because turtles wrote AppKit and I copied AppKit headers. I should have copied UIKit headers, knowing that lemurs wrote UIKit. /cries in corner.

English: Yeah, go blame AppKit headers as I fix this issue.

CodaFi · 2012-12-28T00:17:53Z

There appears to be a noticeable delay between when the slider is moved, and when the cursor changes to the east-west image. Any idea why?

CodaFi · 2012-12-28T00:18:31Z

ExampleProject/Example/ExampleTableViewCell.m

@@ -42,6 +42,9 @@ - (id)initWithStyle:(TUITableViewCellStyle)style reuseIdentifier:(NSString *)reu
 		_textRenderer.shadowColor = [NSColor whiteColor];
 		_textRenderer.shadowOffset = CGSizeMake(0, 1);

+		TUISlider *slider = [[TUISlider alloc] initWithFrame:CGRectMake(5, 5, 250, 32 - 5)];


This frame overlaps the text in the cell pretty bad, especially when tick marks are on. Also, 32-5?

avaidyam · 2012-12-28T00:18:45Z

@CodaFi "east-west image"? I don't see much lag on my MBP... could you explain what you're seeing in more detail? :) Please and thank you!

avaidyam · 2012-12-28T00:23:25Z

@CodaFi I've made the changes you've indicated. (On that note, merry late christmas?)

jwilling · 2012-12-28T01:11:42Z

lib/UIKit/TUISlider.m

+
+ (TUIExtendedSliderCell *)sharedGraphicsRenderer {
+	static TUIExtendedSliderCell *_backingCell;
+	if(_backingCell == nil)


This is subject to race conditions. Please use dispatch_once instead.

@jwilling Fixed!

avaidyam · 2012-12-28T01:20:24Z

@jwilling @CodaFi Any idea on how to animate this kind of a view? I know custom animations are possible with a CALayer animation, but how would I do it with a TUIView/UIView? Would a CVDisplayLink work?

jwilling · 2012-12-28T02:01:49Z

What do you mean by animate? What is there to animate, exactly?

jwilling · 2012-12-28T02:03:15Z

lib/UIKit/TUISlider.m

+
+	// Set up the graphics renderer before drawing. Although it
+	// seems like a burden, it takes around 25ms to configure.
+	[slider setControlSize:(NSControlSize)self.controlSize];


Although it might take 25ms to set the properties, we can't be sure that this will not cause an invalidation of some key aspect of the cell, thereby slowing down the drawing.

This probably means that each of the properties will need to have their setters overridden to forward it onto the cell itself.

@jwilling The reason I set it in the drawing method was so that the "private graphics renderer" would be isolated from any subclasses interested in drawing. What we could do is check if the properties were "dirtied" and if they were, then set everything again.

What do you mean by "isolated"? How does setting it here vs. modifying the setters affect anything?

If subclasses are going to modify the drawing (very much presumably by not using an NSSliderCell to do it), then setting these properties is a waste. My point here is not to re-create an NSSlider wrapper over NSSliderCell, but to simply use it for graphics rendering, hence sharedGraphicsRenderer being a class method. There's only one of these for all TUIKit framework existence because I don't want another NSControl/NSCell mess. If I did, I'd be using AppKit.

Okay then, please check if these properties are different than what the current values is before setting them.

But that again might take longer... the real implication here is that this sharedGraphicsRenderer is "class-pervasive", which means there's only one for all sliders on screen. That's why we need to set these properties each time. Each slider might be differently rendered, but only one cell is taking care of them (I guess AppKit developers who decided on the NSCell segregation are quite happy....?)

avaidyam · 2012-12-28T02:21:05Z

@jwilling I wanted to animate the slider min/max/value properties, like UISlider does. This is possible with CVDisplayLink but that just disregards any sort of TUIView animation properties (curve, duration, etc).

jwilling · 2012-12-28T02:22:47Z

CVDisplayLink is drastic and shouldn't be used lightly. These UISlider animations occur when wrapped in a UIView animation block, you say?

avaidyam · 2012-12-28T02:26:01Z

Agreed, which is why I'm puzzled as to implementing custom TUIView animations. I'm not sure if UISlider animations are implicit UIView animations, given there being ...animated: setter methods. I would very much like if it were possible in TUIView to allow implicit animations when inside animation contexts, however. 😄

jwilling · 2012-12-28T02:28:33Z

lib/UIKit/TUISlider.m

+	[slider setDoubleValue:self.value];
+	[slider setKnobThickness:self.knobThickness];
+	[slider setNumberOfTickMarks:self.numberOfTickMarks];
+	[slider setTickMarkPosition:(NSTickMarkPosition)self.drawTickMarksOnAlternateSide];


This cast should not be here and could be potentially unsafe. Evaluate the current setting and pass in the appropriate type.

jwilling · 2012-12-28T02:47:25Z

lib/UIKit/TUISlider.m

+
+// Defer the drawing, but allow the cell to cache the rects.
+- (void)calcDrawInfo:(NSRect)rect {
+	[NSImage tui_imageWithSize:rect.size drawing:^(CGContextRef ctx) {


What is this? You're not only ignoring the return value of that function (and thereby abusing it), but you're also drawing into nothing.

Yes, I am indeed abusing it and drawing into nothing: if you look above where the track is drawn, I need the slider's knobRect to highlight the slider at exactly the location of the knob. This knobRect is only passed (below in drawKnob:) after drawWithFrame:inView: is called, which is why I draw into nothing. The cell's drawing is deferred to allow the knobRect to "propagate". It's really not a performance hit, but I agree it is a massive abuse of API. However, as justification, that's the only way the knob can be drawn highlighted.

I'd also like to add that this is only possible because NSCell instances are supposed to be highly efficient at drawing, otherwise, the point to cell-backed NSTableViews is lost. So, rest assured, this shouldn't cause a massive gaping hole in drawing speeds.

avaidyam · 2012-12-28T20:51:43Z

@jwilling @CodaFi Any other issues?

jwilling · 2012-12-28T20:52:31Z

@galaxas0 Please don't be impatient; this hasn't been forgotten.

avaidyam · 2012-12-28T20:53:25Z

@jwilling I'm sorry! 🐼 I'll just let it sit then. I'm just trying to get all my PRs through faster... :P

jspahrsummers · 2012-12-29T02:00:01Z

👎 The use of NSCell here is extremely hacky and liable to break in future OS updates. Having to jump through hoops like that at all defeats much of the purpose of TwUI, which is to avoid a lot of the crappy behaviors of AppKit.

It would make more much sense, IMO, to use a TUIViewNSViewContainer and put a real NSSlider in there. If you want customization, why not subclass the real class?

avaidyam · 2012-12-29T02:02:41Z

@jspahrsummers The behavior of NSCell in regards to NSSliderCell has not changed for many years now: I'm not doing anything special except drawing it twice to calculate where the knob is (so highlighting works). The TUIViewNSViewContainer has many flaws (clipping and focus rings being two) that keep me away from using it.

jspahrsummers · 2012-12-29T02:07:52Z

The behavior of NSCell in regards to NSSliderCell has not changed for many years now

That doesn't say anything about whether it might break in the future.

I'm not doing anything special except drawing it twice to calculate where the knob is (so highlighting works).

This is what's hacky.

The TUIViewNSViewContainer has many flaws (clipping and focus rings being two) that keep me away from using it.

Adding more direct uses of AppKit to avoid bridging bugs doesn't help fix those bugs, and contradicts the purpose of the framework.

avaidyam · 2012-12-29T02:09:20Z

Alright, understood. Does this go for the TUIButton PR as well?

jspahrsummers · 2012-12-29T02:17:10Z

Let's talk about that in #149 so that the comments are properly contextual.

avaidyam · 2012-12-29T02:17:56Z

👍

avaidyam added 2 commits December 19, 2012 21:04

TUISlider

60863ac

API completion and parity.

54b4fb9

avaidyam added 3 commits December 22, 2012 13:04

Fire control actions and wrap values properly.

644300a

Merge branch 'master' into upstream-slider

d876024

Verbosity.

066f754

ghost assigned CodaFi Dec 27, 2012

CodaFi reviewed Dec 28, 2012
View reviewed changes

Style fixes.

feebda3

jwilling reviewed Dec 28, 2012
View reviewed changes

Added slider control sizes. (And fixed a race condition).

92ee7f2

jwilling reviewed Dec 28, 2012
View reviewed changes

Add proper control sizing. (And some fixes.)

bacdadb

jwilling reviewed Dec 28, 2012
View reviewed changes

jspahrsummers closed this Dec 29, 2012

avaidyam unassigned CodaFi Nov 11, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TUISlider #151

TUISlider #151

avaidyam commented Dec 20, 2012

jwilling commented Dec 20, 2012

avaidyam commented Dec 21, 2012

CodaFi Dec 28, 2012

avaidyam Dec 28, 2012

CodaFi commented Dec 28, 2012

CodaFi Dec 28, 2012

avaidyam commented Dec 28, 2012

avaidyam commented Dec 28, 2012

jwilling Dec 28, 2012

avaidyam Dec 28, 2012

avaidyam commented Dec 28, 2012

jwilling commented Dec 28, 2012

jwilling Dec 28, 2012

avaidyam Dec 28, 2012

jwilling Dec 28, 2012

avaidyam Dec 28, 2012

jwilling Dec 28, 2012

avaidyam Dec 28, 2012

avaidyam commented Dec 28, 2012

jwilling commented Dec 28, 2012

avaidyam commented Dec 28, 2012

jwilling Dec 28, 2012

jwilling Dec 28, 2012

avaidyam Dec 28, 2012

avaidyam Dec 28, 2012

avaidyam commented Dec 28, 2012

jwilling commented Dec 28, 2012

avaidyam commented Dec 28, 2012

jspahrsummers commented Dec 29, 2012

avaidyam commented Dec 29, 2012

jspahrsummers commented Dec 29, 2012

avaidyam commented Dec 29, 2012

jspahrsummers commented Dec 29, 2012

avaidyam commented Dec 29, 2012

TUISlider #151

TUISlider #151

Conversation

avaidyam commented Dec 20, 2012

jwilling commented Dec 20, 2012

avaidyam commented Dec 21, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodaFi commented Dec 28, 2012

Choose a reason for hiding this comment

avaidyam commented Dec 28, 2012

avaidyam commented Dec 28, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avaidyam commented Dec 28, 2012

jwilling commented Dec 28, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avaidyam commented Dec 28, 2012

jwilling commented Dec 28, 2012

avaidyam commented Dec 28, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avaidyam commented Dec 28, 2012

jwilling commented Dec 28, 2012

avaidyam commented Dec 28, 2012

jspahrsummers commented Dec 29, 2012

avaidyam commented Dec 29, 2012

jspahrsummers commented Dec 29, 2012

avaidyam commented Dec 29, 2012

jspahrsummers commented Dec 29, 2012

avaidyam commented Dec 29, 2012