Bug 147405 - Checkboxes and radio buttons should scale when zooming or pinching to zoom
Summary: Checkboxes and radio buttons should scale when zooming or pinching to zoom
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-29 08:20 PDT by Wenson Hsieh
Modified: 2015-08-05 09:19 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.42 KB, patch)
2015-07-29 10:13 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (10.57 KB, patch)
2015-07-31 15:01 PDT, Wenson Hsieh
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2015-07-29 08:20:04 PDT
When zooming (cmd +/-) or pinching to zoom with the trackpad, checkboxes and radio buttons should scale up in size instead of remaining at a fixed size.
Comment 1 Wenson Hsieh 2015-07-29 08:20:51 PDT
<rdar://problem/20438171>
Comment 2 Wenson Hsieh 2015-07-29 10:13:02 PDT
Created attachment 257754 [details]
Patch
Comment 3 Wenson Hsieh 2015-07-29 12:16:36 PDT
Comment on attachment 257754 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257754&action=review

> Source/WebCore/platform/mac/ThemeMac.mm:412
> +        imageBuffer.release();

This shouldn't be necessary. What is going on here?
Comment 4 Wenson Hsieh 2015-07-29 13:46:13 PDT
Holding off on this until we understand why latter elements of the same size paint to existing IOSurfaces and make us render things incorrectly.
Comment 5 Tim Horton 2015-07-29 13:54:44 PDT
(In reply to comment #3)
> Comment on attachment 257754 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257754&action=review
> 
> > Source/WebCore/platform/mac/ThemeMac.mm:412
> > +        imageBuffer.release();
> 
> This shouldn't be necessary. What is going on here?

"Shouldn't be necessary" -> "is a leak".
Comment 6 Wenson Hsieh 2015-07-31 13:26:59 PDT
This will be unblocked when http://trac.webkit.org/changeset/181301 is rolled out.
Comment 7 Wenson Hsieh 2015-07-31 15:01:11 PDT
Created attachment 257964 [details]
Patch
Comment 8 Dean Jackson 2015-08-04 13:36:08 PDT
Comment on attachment 257964 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257964&action=review

> Source/WebCore/ChangeLog:15
> +        (WebCore::paintToggleButton): Refactored to use drawCellOrFocusRingWithViewOntoContext.
> +        (WebCore::paintButton): Refactored to use drawCellOrFocusRingWithViewOntoContext.

Onto -> Into or With?

> Source/WebCore/platform/mac/ThemeMac.mm:681
> +        std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::createCompatibleBuffer(inflatedRect.size() + 2 * FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth), deviceScaleFactor, ColorSpaceDeviceRGB, context, false);
> +        {
> +            LocalCurrentGraphicsContext localContext(imageBuffer->context());
> +            if (drawButtonCell)
> +                [cell drawWithFrame:imageBufferDrawRect inView:view];
> +            
> +            if (drawFocusRing)
> +                needsRepaint = drawCellFocusRing(cell.get(), imageBufferDrawRect, view);
> +        }
> +        context->drawImageBuffer(imageBuffer.get(), ColorSpaceDeviceRGB, inflatedRect.location() - FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth));

We should ask Tim here, but I wonder if we want ColorSpaceSRGB.
Comment 9 Wenson Hsieh 2015-08-04 14:02:49 PDT
Comment on attachment 257964 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257964&action=review

>> Source/WebCore/platform/mac/ThemeMac.mm:681
>> +        context->drawImageBuffer(imageBuffer.get(), ColorSpaceDeviceRGB, inflatedRect.location() - FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth));
> 
> We should ask Tim here, but I wonder if we want ColorSpaceSRGB.

Couldn't get to Tim, but I asked Simon and he recommended ColorSpaceSRGB over device.
Comment 10 Tim Horton 2015-08-04 14:10:08 PDT
(In reply to comment #9)
> Comment on attachment 257964 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257964&action=review
> 
> >> Source/WebCore/platform/mac/ThemeMac.mm:681
> >> +        context->drawImageBuffer(imageBuffer.get(), ColorSpaceDeviceRGB, inflatedRect.location() - FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth));
> > 
> > We should ask Tim here, but I wonder if we want ColorSpaceSRGB.
> 
> Couldn't get to Tim, but I asked Simon and he recommended ColorSpaceSRGB
> over device.

It is somewhat surprising to start using ColorSpaceSRGB before we switch it everywhere, but it doesn't matter either way.
Comment 11 Wenson Hsieh 2015-08-05 09:03:25 PDT
Committed r187961: <http://trac.webkit.org/changeset/187961>
Comment 12 Wenson Hsieh 2015-08-05 09:19:39 PDT
Build fix in r187963