Bug 143758 – WebCore uses M_PI_2 instead of piOverTwoDouble

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[LRN]

This doesn't work well when M_PI_2 is undefined.

[LRN]

Created attachment 250803 [details]
Use piOverTwoDouble instead of M_PI_2

Because M_PI_2 is not defined everywhere.
Fixes errors like:
  CXX      Source/WebCore/platform/graphics/freetype/libPlatformGtk_la-FontPlatformDataFreeType.lo
../webkitgtk-2.4.8/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp: In function 'void WebCore::rotateCairoMatrixForVerticalOrientation(cairo_matrix_t*)':
../webkitgtk-2.4.8/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:123:34: error: 'M_PI_2' was not declared in this scope
     cairo_matrix_rotate(matrix, -M_PI_2);
                                  ^
GNUmakefile:51663: recipe for target 'Source/WebCore/platform/graphics/freetype/libPlatformGtk_la-FontPlatformDataFreeType.lo' failed

[Darin Adler]

All these patches you uploaded are wrong; they all have No new tests (OOPS!) in them. It was a big mistake to upload tons of patches all at once before you tried one or two and learned how to do them correctly.

[Milan Crha]

I do not need this change with webkit-2.4 branch at revision 182543

[Milan Crha]

(In reply to comment #3)
> I do not need this change with webkit-2.4 branch at revision 182543

Oops, no, I was wrong, it's still in use here. I'm sorry for confusion.

[Milan Crha]

I also patch the same file with this, to be able to build it in webkit-2.4 branch at revision 182543:

--- webkitgtk-2.4.9.orig/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp	2015-04-08 16:19:51 +0000
+++ webkitgtk-2.4.9/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp	2015-04-08 17:11:16 +0000
@@ -32,6 +32,7 @@
 #include <ft2build.h>
 #include FT_TRUETYPE_TABLES_H
 #include <wtf/text/WTFString.h>
+#include <wtf/MathExtras.h>
 
 #if !PLATFORM(EFL)
 #include <gdk/gdk.h>

[LRN]

(In reply to comment #5)
> I also patch the same file with this, to be able to build it in webkit-2.4
> branch at revision 182543:
> 
> ---
> webkitgtk-2.4.9.orig/Source/WebCore/platform/graphics/freetype/
> FontPlatformDataFreeType.cpp	2015-04-08 16:19:51 +0000
> +++
> webkitgtk-2.4.9/Source/WebCore/platform/graphics/freetype/
> FontPlatformDataFreeType.cpp	2015-04-08 17:11:16 +0000
> @@ -32,6 +32,7 @@
>  #include <ft2build.h>
>  #include FT_TRUETYPE_TABLES_H
>  #include <wtf/text/WTFString.h>
> +#include <wtf/MathExtras.h>
>  
>  #if !PLATFORM(EFL)
>  #include <gdk/gdk.h>

What error do you get when you compile without this change? Do you apply any other math-related patches (apart from attachment 250803 [details] or its equivalent) that might need this include?

[Milan Crha]

This is all what I apply on top of webkitgtk3 2.4.4, which is mostly applicable on top of webkit-2.4 branch too (some chunks fail due to being applied already in the branch):

https://git.gnome.org/browse/evolution/tree/win32/patches/webkitgtk.patch

[LRN]

(In reply to comment #7)
> This is all what I apply on top of webkitgtk3 2.4.4, which is mostly
> applicable on top of webkit-2.4 branch too (some chunks fail due to being
> applied already in the branch):
> 
> https://git.gnome.org/browse/evolution/tree/win32/patches/webkitgtk.patch

I haven't asked "what patches are you applying".

I've asked "what error do you get when this particular patch (from https://bugs.webkit.org/show_bug.cgi?id=143758#c5 ) is not applied"?

This should help greatly in pushing these patches upstream.

[LRN]

(In reply to comment #8)
> (In reply to comment #7)
> > This is all what I apply on top of webkitgtk3 2.4.4, which is mostly
> > applicable on top of webkit-2.4 branch too (some chunks fail due to being
> > applied already in the branch):
> > 
> > https://git.gnome.org/browse/evolution/tree/win32/patches/webkitgtk.patch
> 
> I haven't asked "what patches are you applying".
> 
> I've asked "what error do you get when this particular patch (from
> https://bugs.webkit.org/show_bug.cgi?id=143758#c5 ) is not applied"?
> 
> This should help greatly in pushing these patches upstream.

OK, sorry, that does answer the other half of my question. You don't seem to add any other math-related patches to FontPlatformDataFreeType, and changes to math headers are minimal. So there must be a codepath that is activated in your build but not in mine. Which brings us back to the first half of the question - what error are you getting?

[Milan Crha]

(In reply to comment #6)
> > +#include <wtf/MathExtras.h>
> 
> What error do you get when you compile without this change?

Hmm, it builds without it too. I do not know why I added it there, possibly because I prefer explicit includes, than implicit.

[Darin Adler]

(In reply to comment #10)
> (In reply to comment #6)
> > > +#include <wtf/MathExtras.h>
> > 
> > What error do you get when you compile without this change?
> 
> Hmm, it builds without it too. I do not know why I added it there, possibly
> because I prefer explicit includes, than implicit.

OK. We’d like you to follow our project coding style guideline of not doing explicit includes.

[Carlos Garcia Campos]

Committed to 2.4 http://trac.webkit.org/changeset/184550