[webkit-reviews] review denied: [Bug 68321] [EFL] Use C++ type cast instead of C style type cast : [Attachment 108604] Patch

[bugzilla-daemon at webkit.org]

Lucas De Marchi <demarchi at webkit.org> has denied Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 68321: [EFL] Use C++ type cast instead of C style type cast
https://bugs.webkit.org/show_bug.cgi?id=68321

Attachment 108604: Patch
https://bugs.webkit.org/attachment.cgi?id=108604&action=review

------- Additional Comments from Lucas De Marchi <demarchi at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108604&action=review


Please, check the unneeded casts. Especially the float -> double conversions.

> Source/WebKit/efl/ewk/ewk_view.cpp:929
> -	   px = (double)(x + cx) / (w + sd->view.w);
> +	   px = static_cast<double>((x + cx) / (w + sd->view.w));

Unneeded cast.

> Source/WebKit/efl/ewk/ewk_view.cpp:934
> -	   py = (double)(y + cy) / (h + sd->view.h);
> +	   py = static_cast<double>((y + cy) / (h + sd->view.h));

ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1052
> -    int available_width = (int)
priv->page->chrome()->client()->pageRect().width();
> -    int available_height = (int)
priv->page->chrome()->client()->pageRect().height();
> +    int available_width =
static_cast<int>(priv->page->chrome()->client()->pageRect().width());
> +    int available_height =
static_cast<int>(priv->page->chrome()->client()->pageRect().height());

/me wondering why we are going priv->page->chrome()->client() route when we
have the ewk_view available. It would be just a matter of calling
ewk_view_page_rect_get().

About the casts, we are probably better off using enclosingIntRect() instead.

> Source/WebKit/efl/ewk/ewk_view.cpp:1055
> -    int device_width = (int)
priv->page->chrome()->client()->windowRect().width();
> -    int device_height = (int)
priv->page->chrome()->client()->windowRect().height();
> +    int device_width =
static_cast<int>(priv->page->chrome()->client()->windowRect().width());
> +    int device_height =
static_cast<int>(priv->page->chrome()->client()->windowRect().height());

ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1564
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f",
static_cast<float>(priv->settings.zoom_range.min_scale),
static_cast<float>(zoom));

Unneeded casts.

> Source/WebKit/efl/ewk/ewk_view.cpp:1568
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f",
static_cast<float>(priv->settings.zoom_range.max_scale),
static_cast<float>(zoom));

Unneeded casts.

> Source/WebKit/efl/ewk/ewk_view.cpp:1608
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f",
static_cast<float>(priv->settings.zoom_range.min_scale),
static_cast<float>(zoom));

Unneeded casts.

> Source/WebKit/efl/ewk/ewk_view.cpp:1612
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f",
static_cast<float>(priv->settings.zoom_range.max_scale),
static_cast<float>(zoom));

Unneeded casts.

> Source/WebKit/efl/ewk/ewk_view.cpp:1664
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f",
static_cast<float>(priv->settings.zoom_range.min_scale),
static_cast<float>(zoom));

Unneeded casts.

> Source/WebKit/efl/ewk/ewk_view.cpp:1668
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f",
static_cast<float>(priv->settings.zoom_range.max_scale),
static_cast<float>(zoom));

Unneeded casts.