Bug 98979 – [Chromium] Improve vertical text rendering of HarfBuzzShaper

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Kenichi Ishibashi]

- Specify 'vert' and 'vrt2' features when we render vertical text.
- Set appropriate script so that harfbuzz-ng can use the features.

This way, we will see the proper vertical text rendering when we specify 'text-rendering: optimizeLegibility'.

[Kenichi Ishibashi]

Created attachment 168124 [details]
Patch

[Kenichi Ishibashi]

Created attachment 168125 [details]
current expected result

[Kenichi Ishibashi]

Created attachment 168126 [details]
result after applied the patch

[Kenichi Ishibashi]

Hi Tony,

Could you take a look? I think we should resolve vertical text rendering issue on complex text path before transition.

[Tony Chang]

Comment on attachment 168124 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +        (WebCore::findScriptForVerticalGlyphSubstitution): Added/

Nit: / instead of .

> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzNGFace.cpp:103
> +    static const hb_tag_t vertTag = HB_TAG('v', 'e', 'r', 't');
> +    static const hb_tag_t vrt2Tag = HB_TAG('v', 'e', 't', '2');

Nit: It would be nice if we could share this definition.  Maybe make them public static variables on HarfBuzzNGFace?

[Kenichi Ishibashi]

Created attachment 168295 [details]
Patch for landing

[Kenichi Ishibashi]

Created attachment 168296 [details]
Patch for landing

[Kenichi Ishibashi]

Comment on attachment 168124 [details]
Patch

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

Thank you for review!

>> Source/WebCore/ChangeLog:16
>> +        (WebCore::findScriptForVerticalGlyphSubstitution): Added/
> 
> Nit: / instead of .

Done.

>> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzNGFace.cpp:103
>> +    static const hb_tag_t vrt2Tag = HB_TAG('v', 'e', 't', '2');
> 
> Nit: It would be nice if we could share this definition.  Maybe make them public static variables on HarfBuzzNGFace?

Done.

[WebKit Review Bot]

Comment on attachment 168296 [details]
Patch for landing

Clearing flags on attachment: 168296

Committed r131109: <http://trac.webkit.org/changeset/131109>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

This doesn't compile on chromium-mac.

[Adam Barth]

    /Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../third_party/llvm-build/Release+Asserts/bin/clang -x c++ -arch i386 -fmessage-length=0 -pipe -Wno-trigraphs -fno-exceptions -fno-rtti -O0 -Werror -Wnewline-eof -DCHROMIUM_BUILD -DCOMPONENT_BUILD -DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURATION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_HIDPI=1 -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_EXTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DWEBKIT_IMPLEMENTATION=1 -DWEBKIT_DLL "-DWEBCORE_NAVIGATOR_VENDOR=\"Google Inc.\"" -DWEBCORE_NAVIGATOR_PLATFORM="MacIntel" -DWebCascadeList=ChromiumWebCoreObjCWebCascadeList -DWebCoreFlippedView=ChromiumWebCoreObjCWebCoreFlippedView -DWebCoreTextFieldCell=ChromiumWebCoreObjCWebCoreTextFieldCell -DWebScrollbarPrefsObserver=ChromiumWebCoreObjCWebScrollbarPrefsObserver -DWebCoreRenderThemeNotificationObserver=ChromiumWebCoreObjCWebCoreRenderThemeNotificationObserver -DWebFontCache=ChromiumWebCoreObjCWebFontCache -DWebScrollAnimationHelperDelegate=ChromiumWebCoreObjCWebScrollAnimationHelperDelegate -DWebScrollbarPainterControllerDelegate=ChromiumWebCoreObjCWebScrollbarPainterControllerDelegate -DWebScrollbarPainterDelegate=ChromiumWebCoreObjCWebScrollbarPainterDelegate -DWebScrollbarPartAnimation=ChromiumWebCoreObjCWebScrollbarPartAnimation -DENABLE_3D_PLUGIN=1 -DENABLE_BATTERY_STATUS=0 -DENABLE_BLOB=1 -DENABLE_BLOB_SLICE=1 -DENABLE_CHANNEL_MESSAGING=1 -DENABLE_CSP_NEXT=1 -DENABLE_CSS3_TEXT_DECORATION=0 -DENABLE_CSS_BOX_DECORATION_BREAK=1 -DENABLE_CSS_COMPOSITING=0 -DENABLE_CSS_EXCLUSIONS=1 -DENABLE_CSS_FILTERS=1 -DENABLE_CSS_HIERARCHIES=0 -DENABLE_CSS_IMAGE_SET=1 -DENABLE_CSS_IMAGE_RESOLUTION=0 -DENABLE_CSS_REGIONS=1 -DENABLE_CSS_SHADERS=1 -DENABLE_CSS_VARIABLES=1 -DENABLE_CSS_STICKY_POSITION=1 -DENABLE_CUSTOM_SCHEME_HANDLER=0 -DENABLE_DASHBOARD_SUPPORT=0 -DENABLE_DATA_TRANSFER_ITEMS=1 -DENABLE_DETAILS_ELEMENT=1 -DENABLE_DEVICE_ORIENTATION=1 -DENABLE_DIALOG_ELEMENT=1 -DENABLE_DIRECTORY_UPLOAD=1 -DENABLE_DOWNLOAD_ATTRIBUTE=1 -DENABLE_ENCRYPTED_MEDIA=1 -DENABLE_FILE_SYSTEM=1 -DENABLE_FILTERS=1 -DENABLE_FULLSCREEN_API=1 -DENABLE_GAMEPAD=1 -DENABLE_GEOLOCATION=1 -DENABLE_GESTURE_EVENTS=1 -DENABLE_ICONDATABASE=0 -DENABLE_IFRAME_SEAMLESS=1 -DENABLE_INDEXED_DATABASE=1 -DENABLE_INPUT_TYPE_DATE=1 -DENABLE_INPUT_TYPE_DATETIME=1 -DENABLE_INPUT_TYPE_DATETIMELOCAL=1 -DENABLE_INPUT_TYPE_MONTH=1 -DENABLE_INPUT_TYPE_TIME=1 -DENABLE_INPUT_TYPE_WEEK=1 -DENABLE_JAVASCRIPT_DEBUGGER=1 -DENABLE_LEGACY_CSS_VENDOR_PREFIXES=0 -DENABLE_LEGACY_VIEWPORT_ADAPTION=1 -DENABLE_LEGACY_VENDOR_PREFIXES=0 -DENABLE_LEGACY_WEB_AUDIO=1 -DENABLE_LINK_PREFETCH=1 -DENABLE_LINK_PRERENDER=1 -DENABLE_MATHML=1 -DENABLE_MEDIA_SOURCE=1 -DENABLE_MEDIA_STATISTICS=1 -DENABLE_METER_ELEMENT=1 -DENABLE_MHTML=1 -DENABLE_MICRODATA=0 -DENABLE_MUTATION_OBSERVERS=1 -DENABLE_NAVIGATOR_CONTENT_UTILS=1 -DENABLE_PAGE_VISIBILITY_API=1 -DENABLE_POINTER_LOCK=1 -DENABLE_PROGRESS_ELEMENT=1 -DENABLE_QUOTA=1 -DENABLE_REQUEST_ANIMATION_FRAME=1 -DENABLE_RUBY=1 -DENABLE_SANDBOX=1 -DENABLE_SCRIPTED_SPEECH=1 -DENABLE_SHADOW_DOM=1 -DENABLE_SMOOTH_SCROLLING=1 -DENABLE_SQL_DATABASE=1 -DENABLE_STYLE_SCOPED=1 -DENABLE_SVG=1 -DENABLE_SVG_FONTS=1 -DENABLE_TEXT_AUTOSIZING=1 -DENABLE_TOUCH_ADJUSTMENT=1 -DENABLE_TOUCH_EVENTS=1 -DENABLE_TOUCH_ICON_LOADING=0 -DENABLE_TOUCH_SLIDER=1 -DENABLE_V8_SCRIPT_DEBUG_SERVER=1 -DENABLE_VIDEO=1 -DENABLE_VIDEO_TRACK=1 -DENABLE_VIEWPORT=1 -DENABLE_WEBGL=1 -DENABLE_WEB_SOCKETS=1 -DENABLE_WEB_TIMING=1 -DENABLE_WIDGET_REGION=1 -DENABLE_WORKERS=1 -DENABLE_XHR_RESPONSE_BLOB=1 -DENABLE_XSLT=1 -DWTF_USE_LEVELDB=1 -DWTF_USE_BUILTIN_UTF8_CODEC=1 -DWTF_USE_OPENTYPE_SANITIZER=1 -DWTF_USE_RTL_SCROLLBAR=1 -DWTF_USE_SKIA_TEXT=1 -DWTF_USE_WEBP=1 -DWTF_USE_WEBKIT_IMAGE_DECODERS=1 -DENABLE_ACCELERATED_OVERFLOW_SCROLLING=0 -DENABLE_CALENDAR_PICKER=1 -DENABLE_DATALIST_ELEMENT=1 -DENABLE_INPUT_SPEECH=1 -DENABLE_INPUT_TYPE_COLOR=1 -DENABLE_INPUT_MULTIPLE_FIELDS_UI=1 -DENABLE_JAVASCRIPT_I18N_API=1 -DENABLE_LEGACY_NOTIFICATIONS=1 -DENABLE_MEDIA_CAPTURE=0 -DENABLE_MEDIA_STREAM=1 -DENABLE_NOTIFICATIONS=1 -DENABLE_ORIENTATION_EVENTS=0 -DENABLE_PAGE_POPUP=1 -DENABLE_SHARED_WORKERS=1 -DENABLE_WEB_AUDIO=1 -DENABLE_3D_RENDERING=1 -DENABLE_ACCELERATED_2D_CANVAS=1 -DWTF_USE_ACCELERATED_COMPOSITING=1 -DENABLE_RUBBER_BANDING=1 -DWTF_USE_SKIA_ON_MAC_CHROMIUM=1 -DBUILDING_CHROMIUM__=1 -DUSE_SYSTEM_MALLOC=1 -DWTF_USE_NEW_THEME=1 -DU_USING_ICU_NAMESPACE=0 -DGURL_DLL -DSK_BUILD_NO_IMAGE_ENCODE -DSK_DEFERRED_CANVAS_USES_GPIPE=1 -DGR_GL_CUSTOM_SETUP_HEADER="GrGLConfig_chrome.h" -DGR_AGGRESSIVE_SHADER_OPTS=1 -DSK_USE_POSIX_THREADS -DGR_DLL -DSKIA_DLL -DCHROME_PNG_WRITE_SUPPORT -DPNG_USER_CONFIG -DLIBXML_STATIC -DLIBXSLT_STATIC -DV8_SHARED -DUSING_V8_SHARED -D__STDC_FORMAT_MACROS -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -isysroot /Developer/SDKs/MacOSX10.6.sdk -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -mmacosx-version-min=10.6 -gdwarf-2 -Wglobal-constructors -Wunused-parameter -Wall -Wendif-labels -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wheader-hygiene -Wno-char-subscripts -Wno-unused-function -Wno-unnamed-type-template-args -Wno-c++11-extensions -Wno-covered-switch-default -Wno-implicit-conversion-floating-point-to-bool -Wexit-time-destructors -F/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/Debug -I/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/Debug/include -I../../../../icu/public/common -I../../../../icu/public/i18n -I../../../../../third_party/apple_webkit -I../../../../../third_party/khronos -I../../../../.. -I../../Platform/chromium -I/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/WebCore.build/DerivedSources/Debug -I.. -I../.. -I../Modules/battery -I../Modules/filesystem -I../Modules/filesystem/chromium -I../Modules/gamepad -I../Modules/geolocation -I../Modules/intents -I../Modules/indexeddb -I../Modules/mediasource -I../Modules/mediastream -I../Modules/navigatorcontentutils -I../Modules/notifications -I../Modules/quota -I../Modules/speech -I../Modules/webaudio -I../Modules/webdatabase -I../Modules/webdatabase/chromium -I../Modules/websockets -I../accessibility -I../accessibility/chromium -I../bindings -I../bindings/generic -I../bindings/v8 -I../bindings/v8/custom -I../bridge -I../bridge/jni -I../bridge/jni/v8 -I../css -I../dom -I../dom/default -I../editing -I../fileapi -I../history -I../html -I../html/canvas -I../html/parser -I../html/shadow -I../html/track -I../inspector -I../loader -I../loader/appcache -I../loader/archive -I../loader/archive/cf -I../loader/archive/mhtml -I../loader/cache -I../loader/icon -I../mathml -I../page -I../page/animation -I../page/chromium -I../page/scrolling -I../platform -I../platform/animation -I../platform/audio -I../platform/audio/chromium -I../platform/chromium -I../platform/chromium/support -I../platform/graphics -I../platform/graphics/chromium -I../platform/graphics/chromium/cc -I../platform/graphics/filters -I../platform/graphics/filters/arm -I../platform/graphics/gpu -I../platform/graphics/opentype -I../platform/graphics/skia -I../platform/graphics/transforms -I../platform/image-decoders -I../platform/image-decoders/bmp -I../platform/image-decoders/gif -I../platform/image-decoders/ico -I../platform/image-decoders/jpeg -I../platform/image-decoders/png -I../platform/image-decoders/skia -I../platform/image-decoders/webp -I../platform/image-encoders/skia -I../platform/leveldb -I../platform/mediastream -I../platform/mediastream/chromium -I../platform/mock -I../platform/network -I../platform/network/chromium -I../platform/sql -I../platform/text -I../platform/text/transcoder -I../plugins -I../plugins/chromium -I../rendering -I../rendering/mathml -I../rendering/style -I../rendering/svg -I../storage -I../storage/chromium -I../svg -I../svg/animation -I../svg/graphics -I../svg/graphics/filters -I../svg/properties -I../../ThirdParty/glu -I../workers -I../xml -I../xml/parser -I../platform/audio/mac -I../platform/cocoa -I../platform/graphics/cg -I../platform/graphics/cocoa -I../platform/graphics/mac -I../platform/mac -I../platform/text/mac -I../platform/graphics/harfbuzz -I../platform/graphics/harfbuzz/ng -I../../../../../gpu -I../../../../../third_party/angle/include/GLSLANG -I/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/DerivedSources/Debug/webkit -I/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/DerivedSources/Debug/webkit/bindings -I../../WTF -I../../JavaScriptCore -I../../../../../skia/config -I../../../../../third_party/skia/src/core -I../../../../../third_party/skia/include/config -I../../../../../third_party/skia/include/core -I../../../../../third_party/skia/include/effects -I../../../../../third_party/skia/include/pdf -I../../../../../third_party/skia/include/gpu -I../../../../../third_party/skia/include/gpu/gl -I../../../../../third_party/skia/include/pipe -I../../../../../third_party/skia/include/ports -I../../../../../third_party/skia/include/utils -I../../../../../skia/ext -I../../../../../third_party/skia/include/utils/mac -I../../../../iccjpeg -I../../../../libwebp -I../../../../libpng -I../../../../libxml/mac/include -I../../../../libxml/src/include -I../../../../libxslt -I../../../../npapi -I../../../../npapi/bindings -I../../../../ots/include -I../../../../qcms/src -I../../../../sqlite -I../../../../zlib -I../../../../../v8/include -I../../../../libjpeg_turbo -I../../../../leveldatabase/src/include -I../../../../leveldatabase/src -I../../../../harfbuzz-ng/src -I/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/WebCore.build/Debug/webcore_platform.build/DerivedSources/i386 -I/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/WebCore.build/Debug/webcore_platform.build/DerivedSources -fno-strict-aliasing -Xclang -load -Xclang /Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/tools/clang/scripts/../../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang skip-virtuals-in-implementations -Xclang -plugin-arg-find-bad-constructs -Xclang check-cc-directory -fstack-protector-all -include /Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../WebCorePrefix.h -c /Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../platform/graphics/harfbuzz/ng/HarfBuzzNGFace.cpp -o /Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../../../../../xcodebuild/WebCore.build/Debug/webcore_platform.build/Objects-normal/i386/HarfBuzzNGFace.o
/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../platform/graphics/harfbuzz/ng/HarfBuzzNGFace.cpp:116:102:error: use of undeclared identifier 'vertTag'
            if (hb_ot_layout_language_find_feature(face, HB_OT_TAG_GSUB, scriptIndex, languageIndex, vertTag, &featureIndex)
                                                                                                     ^
/Volumes/data/b/build/slave/Mac_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../platform/graphics/harfbuzz/ng/HarfBuzzNGFace.cpp:117:105:error: use of undeclared identifier 'vrt2Tag'
                || hb_ot_layout_language_find_feature(face, HB_OT_TAG_GSUB, scriptIndex, languageIndex, vrt2Tag, &featureIndex))
                                                                                                        ^
2 errors generated.

[WebKit Review Bot]

Re-opened since this is blocked by bug 99124

[Kenichi Ishibashi]

Created attachment 168330 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 168330 [details]
Patch for landing

Clearing flags on attachment: 168330

Committed r131126: <http://trac.webkit.org/changeset/131126>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.