tobihagemann / THLabel

UILabel subclass, which additionally allows shadow blur, inner shadow, stroke text and fill gradient.
https://tobiha.de
Other
665 stars 111 forks source link

crash with swift 3, xcode 8 #40

Closed ConfusedVorlon closed 7 years ago

ConfusedVorlon commented 7 years ago

I'm getting a strange crash in

when I call it from the did set on text (I'm doing some logic in a subclass to provide better multi-line wrapping)

    override var text: String? {
        didSet {
            wrapText()
        }
    }

it is a bad access at

CFStringRef stringRef = (__bridge CFStringRef)self.text;

I can fix it by replacing that with

    NSString *myString=self.text;

    CFStringRef stringRef = (__bridge CFStringRef)myString;

I don't really understand what is happening here - but something memory related...

tobihagemann commented 7 years ago

Thank you for reporting the issue! Could you try this instead?

CFStringRef stringRef = (CFStringRef)CFBridgingRetain(self.text); // this has changed
CFAttributedStringRef attributedStringRef = CFAttributedStringCreate(kCFAllocatorDefault, stringRef, attributes);
CFRelease(stringRef); // this is new
CFRelease(attributes);

Maybe it's necessary to take ownership of self.text. It should be safe to release stringRef immediately, because CFAttributedStringCreate() copies the value of stringRef.

I can't reproduce the issue, that's why I'll wait for your response. If my suggestion is working out for you, I'll release a new version.

ConfusedVorlon commented 7 years ago

yes - that fixes it for me.

I don't understand it either; I can't see why self.text could be released, but that seems to be happening.

btw - thank you for a great label!

ConfusedVorlon commented 7 years ago

on the matter of why I was poking around; Have you considered adding wrapping functionality?

this project seems to do a good job. https://github.com/tbaranes/FittableFontLabel I have hacked something together subclassing THLabel, but it isn't totally correct.

I'm loving how changing the kerning can give me this comic effect: 2016-09-16 10 16 39 pm

tobihagemann commented 7 years ago

Yeah, I'm also confused why it's happening in your case. All right, I'll prepare a small update then! :wink:

I haven't looked into wrapping functionality too closely, because Core Text doesn't support it and I never had the need for this. I would assume that the solution would be to do it "manually". I've looked briefly into FittableFontLabel and it looks like that the binarySearch() function is doing the job. I assume that this recursion has some performance impact (should be negligible in most cases, but it's probably really bad for games). I'm not an expert on text rendering and I really have no idea on how to do it better. I know that THLabel isn't respecting all properties of UILabel and this could be improved, but I don't think I'll find the time for it. 😅

ConfusedVorlon commented 7 years ago

thanks for doing an update. It's always nicer to be able to use a clean cocoapod.

re wrapping; that's fair enough - I'm using the binary search on my subclass of THLabel. there is certainly cost - but if numberOfLines == 1, then you can skip the wrapping, so it is only paid when needed.

I'll show my code below. I clearly have some error though as I'm having to hack in some extra padding...

class WrappingTHLabel : THLabel {

    var maxFontSize:CGFloat=60
    var minFontSize:CGFloat=10

    var lastWrappedText:String?
    var lastTextWrappedSize:CGSize = CGSize.zero

    override var text: String? {
        didSet {
            wrapText()
        }
    }

    override func layoutSubviews() {
        super.layoutSubviews()
        wrapText()
    }

    func fits() -> Bool {
        let intrinsic = self.intrinsicContentSize
        let bounds = self.bounds.size

        //hack to deal with reality...
        let padding:CGFloat = 10.0

        if ((intrinsic.width + padding) > bounds.width || intrinsic.height > bounds.height){
            return false
        }

        return true
    }

    func wrapText() {
        if lastTextWrappedSize == self.bounds.size && self.text == lastWrappedText{
            return
        }

        lastTextWrappedSize=self.bounds.size
        lastWrappedText=self.text

        self.font = self.font.withSize(maxFontSize)
        if (fits()) {
            return;
        }

        binaryFontSearch(minSize: minFontSize, maxSize: maxFontSize)

    }

    func binaryFontSearch(minSize:CGFloat, maxSize:CGFloat){
        //print("search \(minSize) -> \(maxSize)")

        let newSize = (minSize + maxSize)/2
        self.font = self.font.withSize(newSize)

        let nowFits = fits()

        //stop looking when down to 1pt difference
        let diff = maxSize - newSize
        if (diff < 1.0){
            if nowFits {
                //print("finishing with fit: \(newSize)")
                return
            }
            else {
                //print("finishing not fitting: \(minSize)")
                self.font = self.font.withSize(minSize)
                return
            }
        }

        if nowFits {
            binaryFontSearch(minSize: newSize, maxSize: maxSize)
        }
        else {
            binaryFontSearch(minSize: minSize, maxSize: newSize)
        }

    }

}
tobihagemann commented 7 years ago

Fixed with 9fed07381ed629d44199c1ac52e47adf7362a7a8

Thanks for sharing! Not sure about the padding. Have you read the paragraph on Text Insets / Padding? Anyway, I'm closing this issue because the original problem has been fixed.

ConfusedVorlon commented 7 years ago

I did look at that section. My reading is that if I stick with defaults, then THLabel will automatically add sufficient insets. This should mean that if the intrinsic content size is less than the actual size - then everything draws correctly. I practice - it needs a bit more padding!

Anyway - thanks for fixing this issue so quickly.