Bug 46029 – [EFL] add pango support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

In order to render complex text, we can choose pango as option.

[Ryuan Choi]

Created attachment 68000 [details]
Patch

[Rafael Antognolli]

Hi Ryuan,

Do you really need to include the gtk directory here?

+IF (WTF_USE_PANGO)
+  LIST(APPEND WebKit_INCLUDE_DIRECTORIES
+    "${WEBCORE_DIR}/platform/graphics/gtk"
+    ${Pango_INCLUDE_DIRS}
+  )
+  LIST(APPEND WebKit_LIBRARIES
+    ${Pango_LIBRARIES}

And maybe you should also remove FIND_PACKAGE(Freetype) from the global scope and just use it when using that backend. Or is it necessary by pango too?

Other than that, the patch seems nice.

[Ryuan Choi]

(In reply to comment #2)
> Hi Ryuan,
> 
> Do you really need to include the gtk directory here?
> 
> +IF (WTF_USE_PANGO)
> +  LIST(APPEND WebKit_INCLUDE_DIRECTORIES
> +    "${WEBCORE_DIR}/platform/graphics/gtk"
> +    ${Pango_INCLUDE_DIRS}
> +  )
> +  LIST(APPEND WebKit_LIBRARIES
> +    ${Pango_LIBRARIES}
> 

sorry about late answer.
now, Pango related implementation was in platform/graphics/gtk.
unless moving it, we should include it.

> And maybe you should also remove FIND_PACKAGE(Freetype) from the global scope and just use it when using that backend. Or is it necessary by pango too?
> 
> Other than that, the patch seems nice.

It's my mistake. I'll move it.

[Ryuan Choi]

Created attachment 68844 [details]
Patch

[Ryuan Choi]

(In reply to comment #4)
> Created an attachment (id=68844) [details]
> Patch

I moved FIND_PACKAGE(Freetype).

[Rafael Antognolli]

(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=68844) [details] [details]
> > Patch
> 
> I moved FIND_PACKAGE(Freetype).

(In reply to comment #4)
> Created an attachment (id=68844) [details]
> Patch

(In reply to comment #3)
> (In reply to comment #2)
> > Hi Ryuan,
> > 
> > Do you really need to include the gtk directory here?
> > 
> > +IF (WTF_USE_PANGO)
> > +  LIST(APPEND WebKit_INCLUDE_DIRECTORIES
> > +    "${WEBCORE_DIR}/platform/graphics/gtk"
> > +    ${Pango_INCLUDE_DIRS}
> > +  )
> > +  LIST(APPEND WebKit_LIBRARIES
> > +    ${Pango_LIBRARIES}
> > 
> 
> sorry about late answer.
> now, Pango related implementation was in platform/graphics/gtk.
> unless moving it, we should include it.

Ok, I understand your point, but would be better to have these files inside somewhere else (maybe platform/graphics/pango?).

Anyway, I think it's ok for now.

[Rafael Antognolli]

Hi mrobinson,

Do you think there's any problem moving these files to some non-gtk specific folder? Currently they don't use anything gtk related...

[Martin Robinson]

I think it's a great idea actually. A followup or pre-patch could:

1. Move all Pango backend files to a "pango" subdirectory.
2. Move all FreeType backend files to a "freetype" subdirectory.
3. Rename all inconsistently named FreeType backend files.

I believe that would allow us to remove at least some of the #ifdef magic from FontPlatformData.h.

[Ryuan Choi]

Created attachment 73698 [details]
Patch

[Ryuan Choi]

(In reply to comment #9)
> Created an attachment (id=73698) [details]
> Patch

Finally this patch can be applied because related bugs was fixed.

could you review this, if possible?

[WebKit Commit Bot]

Comment on attachment 73698 [details]
Patch

Clearing flags on attachment: 73698

Committed r71987: <http://trac.webkit.org/changeset/71987>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.