Bug 73713 - [GTK] TextureMapperNode should not use Qt types and functions
Summary: [GTK] TextureMapperNode should not use Qt types and functions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 73634
  Show dependency treegraph
 
Reported: 2011-12-02 16:01 PST by Alejandro G. Castro
Modified: 2011-12-03 02:09 PST (History)
1 user (show)

See Also:


Attachments
Proposed patch (2.48 KB, patch)
2011-12-02 16:03 PST, Alejandro G. Castro
noam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alejandro G. Castro 2011-12-02 16:01:22 PST
Currently it is using qreal and qMin.
Comment 1 Alejandro G. Castro 2011-12-02 16:03:21 PST
Created attachment 117709 [details]
Proposed patch
Comment 2 Martin Robinson 2011-12-02 16:51:27 PST
Comment on attachment 117709 [details]
Proposed patch

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

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:835
> +        return std::min(1.0, (floor(numSteps * t) + 1) / numSteps);

I'm not certain, but I think its more typical to include MathExtras.h here and just use min.
Comment 3 Noam Rosenthal 2011-12-02 18:17:47 PST
Comment on attachment 117709 [details]
Proposed patch

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

>> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:835
>> +        return std::min(1.0, (floor(numSteps * t) + 1) / numSteps);
> 
> I'm not certain, but I think its more typical to include MathExtras.h here and just use min.

True. Please fix before committing :)
Comment 4 Alejandro G. Castro 2011-12-03 01:39:27 PST
I've talked to Martin we checked that min is not directly included by MathExtras, he recommended to include it to add Windows support and use std:min.
Comment 5 Alejandro G. Castro 2011-12-03 02:09:45 PST
Landed! Thanks http://trac.webkit.org/changeset/101919