CPTextField sizeToFit may clip text #1400

Closed
walisser opened this Issue Nov 17, 2011 · 17 comments

8 participants

@walisser

You can see in the screenshot the right edge is clipped by one pixel

http://img24.imageshack.us/img24/1391/psimagef.jpg

var slash1 = [[CPTextField alloc] initWithFrame:CGRectMake(23, 7, 15, 18)];
 [slash1 setStringValue:@"/"];
 [slash1 sizeToFit];
 [slash1 setBackgroundColor:[CPColor greenColor]];

Cappuccino 0.9.5
Chrome 16.0, Ubuntu 11.10

And here I've expanded the frame (after sizeToFit) by one pixel
http://img20.imageshack.us/img20/4802/psimageq.jpg

@thewalkingtoast

This is very interesting. Both Firefox and Chrome are reporting a clientWidth of 3px on that element once created so that is the value Cappuccino sets the width property for sizeToFit.

However, the following code shows clearly in both Firefox and Chrome that they are missing a pixel:

var span = document.createElement("span");

var style = span.style;
    style.position = "absolute";
    style.visibility = "visible";
    style.padding = "0px";
    style.margin = "0px";
    style.whiteSpace = "pre";
    style.font = "12px Arial,sans-serif";
    style.backgroundColor = "#88CC88";

var body = document.getElementsByTagName("body")[0];

body.appendChild(span);

var t = span;

t.textContent = "\\";
t.style.width = t.clientWidth + "px";

console.log(t.clientWidth);

This method is similar to how Cappuccino determines clientWidth as seen in AppKit/Platform/DOM/CPPlatformString.j.

Try replacing the textContent value with say, the number '6' and you will it is accurate but either of the slashes (/ or \) lose a pixel.

@cappbot

Label: #new. What's next? A reviewer should examine this issue.

@schipmolder

#needs-reduction
+AppKit
+bug

@cappbot

Labels: #needs-reduction, #new, AppKit, bug. What's next? A minimal test app should be created which demonstrates the concern of this issue in isolation.

@ahankinson

-#new
milestone=Someday

@cappbot

Milestone: Someday. Labels: #needs-reduction, AppKit, bug. What's next? A minimal test app should be created which demonstrates the concern of this issue in isolation.

@daboe01

i am seeing something similar in chrome in the latest master:
bildschirmfoto 2014-06-30 um 20 29 13

@daboe01

this is probably caused by not rounding the return value of + (CGSize)sizeOfString:(CPString)aString withFont:(CPFont)aFont forWidth:(float)aWidth.

@aparajita

@daboe01 Couldn't be because of rounding, because the size calculations always return integral values. It has more to do with inconsistencies in rounding done by the browsers internally. The browsers themselves incorrectly size text fields in certain cases because they don't round correctly with fractional pixels. That's why CPImageAndTextView adds 1 pixel to account for incorrect rounding. But it seems Chrome and FireFox broke even that.

@primalmotion
Cappuccino member
@daboe01
@daboe01

@primalmotion please take a look at the sizing function that i wrote for CPTypesetter:

var _didTestCanvasSizingValid,
    _measuringContext,
    _isCanvasSizingInvalid,
    _measuringContextFont;

function _widthOfStringForFont(aString, aFont)
{
    if (!_measuringContext)
        _measuringContext = CGBitmapGraphicsContextCreate();

    if (!_didTestCanvasSizingValid && CPFeatureIsCompatible(CPHTMLCanvasFeature))
    {
        var teststring = "0123456879abcdefghiklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ,.-()";

        _didTestCanvasSizingValid = YES;
        _measuringContext.font = [aFont cssString];
        _isCanvasSizingInvalid = ABS([teststring sizeWithFont:aFont].width -_measuringContext.measureText(teststring)) > 1;
    }

    if (!CPFeatureIsCompatible(CPHTMLCanvasFeature) || _isCanvasSizingInvalid)  // e.g. Safari, whose measureText results differ from [aString sizeWithFont:aFont]
        return [aString sizeWithFont:aFont];

    if (_measuringContextFont !== aFont)
    {
        _measuringContextFont = aFont;
        _measuringContext.font = [aFont cssString];
    }

    return ROUND(_measuringContext.measureText(aString));
}

this might be worth integrating into + (CGSize)sizeOfString:(CPString)aString withFont:(CPFont)aFont forWidth:(float)aWidth because measureText is more accurate (enables rounding instead of reading the truncated DOM property). It is also approx. 10x times faster than the current cappuccino measuring techique that manipulates the DOM.

@aparajita

@daboe01 Looks like a good solution, but of course it currently will only work in environments that have canvas available. In the future we are dropping support for such environments, so this is the best solution going forward.

@daboe01

it will fall back to conventional sizing in non-canvas environments

@daboe01

i do not see this anymore with the latest master.
+#fixed

@cappbot

Milestone: Someday. Labels: #fixed, AppKit, bug. What's next? This issue is considered successfully resolved.

@cappbot cappbot added the #fixed label Nov 26, 2014
@cappbot cappbot closed this Nov 26, 2014
@cappbot

Milestone: Someday. Labels: #fixed, AppKit, bug. What's next? This issue is considered successfully resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment