From 963d7a80adcc23b282ef9d91f6eec9719a9a16df Mon Sep 17 00:00:00 2001 From: Alexis Beingessner Date: Tue, 17 Oct 2017 16:05:29 -0400 Subject: [PATCH] Bug 1407767 - reimplement text bullets to use TextDrawTarget. r=jrmuizel This makes the implementation more robust and unifies all our text handling under one implementation. Also includes some refatoring of our CreateWebRenderCommands impl to divest us from the layerful/stateful GetLayerState(). Future work should be done here, but that's low priority for the moment. MozReview-Commit-ID: Hypb02aDStm --HG-- extra : rebase_source : 161a17a719b8dcd856378a2af96ac3fde66d3aa7 --- layout/generic/nsBulletFrame.cpp | 150 +++++++++++++++---------------- 1 file changed, 74 insertions(+), 76 deletions(-) diff --git a/layout/generic/nsBulletFrame.cpp b/layout/generic/nsBulletFrame.cpp index 9e70fb5fafe0..c1663957d37d 100644 --- a/layout/generic/nsBulletFrame.cpp +++ b/layout/generic/nsBulletFrame.cpp @@ -224,7 +224,7 @@ public: MOZ_ASSERT(IsTextType()); } - void + bool CreateWebRenderCommands(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -269,11 +269,16 @@ public: bool BuildGlyphForText(nsDisplayItem* aItem, bool disableSubpixelAA); + void + PaintTextToContext(nsIFrame* aFrame, + gfxContext* aCtx, + bool aDisableSubpixelAA); + bool IsImageContainerAvailable(layers::LayerManager* aManager, uint32_t aFlags); private: - void + bool CreateWebRenderCommandsForImage(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -281,7 +286,7 @@ private: mozilla::layers::WebRenderLayerManager* aManager, nsDisplayListBuilder* aDisplayListBuilder); - void + bool CreateWebRenderCommandsForPath(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -289,7 +294,7 @@ private: mozilla::layers::WebRenderLayerManager* aManager, nsDisplayListBuilder* aDisplayListBuilder); - void + bool CreateWebRenderCommandsForText(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -322,7 +327,7 @@ private: int32_t mListStyleType; }; -void +bool BulletRenderer::CreateWebRenderCommands(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -331,15 +336,15 @@ BulletRenderer::CreateWebRenderCommands(nsDisplayItem* aItem, nsDisplayListBuilder* aDisplayListBuilder) { if (IsImageType()) { - CreateWebRenderCommandsForImage(aItem, aBuilder, aResources, + return CreateWebRenderCommandsForImage(aItem, aBuilder, aResources, aSc, aManager, aDisplayListBuilder); } else if (IsPathType()) { - CreateWebRenderCommandsForPath(aItem, aBuilder, aResources, + return CreateWebRenderCommandsForPath(aItem, aBuilder, aResources, aSc, aManager, aDisplayListBuilder); } else { MOZ_ASSERT(IsTextType()); - CreateWebRenderCommandsForText(aItem, aBuilder, aResources, aSc, - aManager, aDisplayListBuilder); + return CreateWebRenderCommandsForText(aItem, aBuilder, aResources, aSc, + aManager, aDisplayListBuilder); } } @@ -377,18 +382,7 @@ BulletRenderer::Paint(gfxContext& aRenderingContext, nsPoint aPt, } if (IsTextType()) { - DrawTarget* drawTarget = aRenderingContext.GetDrawTarget(); - DrawTargetAutoDisableSubpixelAntialiasing - disable(drawTarget, aDisableSubpixelAA); - - aRenderingContext.SetColor(Color::FromABGR(mColor)); - - nsPresContext* presContext = aFrame->PresContext(); - if (!presContext->BidiEnabled() && HasRTLChars(mText)) { - presContext->SetBidiEnabled(); - } - nsLayoutUtils::DrawString(aFrame, *mFontMetrics, &aRenderingContext, - mText.get(), mText.Length(), mPoint); + PaintTextToContext(aFrame, &aRenderingContext, aDisableSubpixelAA); } return DrawResult::SUCCESS; @@ -407,21 +401,7 @@ BulletRenderer::BuildGlyphForText(nsDisplayItem* aItem, bool disableSubpixelAA) RefPtr captureCtx = gfxContext::CreateOrNull(capture); - { - DrawTargetAutoDisableSubpixelAntialiasing - disable(capture, disableSubpixelAA); - - captureCtx->SetColor( - Color::FromABGR(mColor)); - - nsPresContext* presContext = aItem->Frame()->PresContext(); - if (!presContext->BidiEnabled() && HasRTLChars(mText)) { - presContext->SetBidiEnabled(); - } - - nsLayoutUtils::DrawString(aItem->Frame(), *mFontMetrics, captureCtx, - mText.get(), mText.Length(), mPoint); - } + PaintTextToContext(aItem->Frame(), captureCtx, disableSubpixelAA); layers::GlyphArray* g = mGlyphs.AppendElement(); std::vector glyphs; @@ -439,6 +419,27 @@ BulletRenderer::BuildGlyphForText(nsDisplayItem* aItem, bool disableSubpixelAA) return true; } +void +BulletRenderer::PaintTextToContext(nsIFrame* aFrame, + gfxContext* aCtx, + bool aDisableSubpixelAA) +{ + MOZ_ASSERT(IsTextType()); + + DrawTarget* drawTarget = aCtx->GetDrawTarget(); + DrawTargetAutoDisableSubpixelAntialiasing + disable(drawTarget, aDisableSubpixelAA); + + aCtx->SetColor(Color::FromABGR(mColor)); + + nsPresContext* presContext = aFrame->PresContext(); + if (!presContext->BidiEnabled() && HasRTLChars(mText)) { + presContext->SetBidiEnabled(); + } + nsLayoutUtils::DrawString(aFrame, *mFontMetrics, aCtx, + mText.get(), mText.Length(), mPoint); +} + bool BulletRenderer::IsImageContainerAvailable(layers::LayerManager* aManager, uint32_t aFlags) { @@ -447,7 +448,7 @@ BulletRenderer::IsImageContainerAvailable(layers::LayerManager* aManager, uint32 return mImage->IsImageContainerAvailable(aManager, aFlags); } -void +bool BulletRenderer::CreateWebRenderCommandsForImage(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -455,27 +456,29 @@ BulletRenderer::CreateWebRenderCommandsForImage(nsDisplayItem* aItem, mozilla::layers::WebRenderLayerManager* aManager, nsDisplayListBuilder* aDisplayListBuilder) { - MOZ_ASSERT(IsImageType()); - - if (!mImage) { - return; - } + MOZ_RELEASE_ASSERT(IsImageType()); + MOZ_RELEASE_ASSERT(mImage); uint32_t flags = aDisplayListBuilder->ShouldSyncDecodeImages() ? imgIContainer::FLAG_SYNC_DECODE : imgIContainer::FLAG_NONE; + // FIXME: is this check always redundant with the next one? + if (IsImageContainerAvailable(aManager, flags)) { + return false; + } + RefPtr container = mImage->GetImageContainer(aManager, flags); if (!container) { - return; + return false; } gfx::IntSize size; Maybe key = aManager->CommandBuilder().CreateImageKey(aItem, container, aBuilder, aResources, aSc, size, Nothing()); if (key.isNothing()) { - return; + return true; // Nothing to do } const int32_t appUnitsPerDevPixel = aItem->Frame()->PresContext()->AppUnitsPerDevPixel(); @@ -487,9 +490,11 @@ BulletRenderer::CreateWebRenderCommandsForImage(nsDisplayItem* aItem, !aItem->BackfaceIsHidden(), wr::ImageRendering::Auto, key.value()); + + return true; } -void +bool BulletRenderer::CreateWebRenderCommandsForPath(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -501,10 +506,13 @@ BulletRenderer::CreateWebRenderCommandsForPath(nsDisplayItem* aItem, if (!aManager->CommandBuilder().PushItemAsImage(aItem, aBuilder, aResources, aSc, aDisplayListBuilder)) { NS_WARNING("Fail to create WebRender commands for Bullet path."); + return false; } + + return true; } -void +bool BulletRenderer::CreateWebRenderCommandsForText(nsDisplayItem* aItem, wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources, @@ -513,32 +521,20 @@ BulletRenderer::CreateWebRenderCommandsForText(nsDisplayItem* aItem, nsDisplayListBuilder* aDisplayListBuilder) { MOZ_ASSERT(IsTextType()); - MOZ_ASSERT(mFont); - MOZ_ASSERT(!mGlyphs.IsEmpty()); - const int32_t appUnitsPerDevPixel = aItem->Frame()->PresContext()->AppUnitsPerDevPixel(); bool dummy; - LayoutDeviceRect destRect = LayoutDeviceRect::FromAppUnits( - aItem->GetBounds(aDisplayListBuilder, &dummy), appUnitsPerDevPixel); - wr::LayoutRect wrDestRect = aSc.ToRelativeLayoutRect(destRect); + nsRect bounds = aItem->GetBounds(aDisplayListBuilder, &dummy); - nsTArray wrGlyphs; - - for (layers::GlyphArray& glyphArray : mGlyphs) { - const auto& glyphs = glyphArray.glyphs(); - wrGlyphs.SetLength(glyphs.Length()); - - for (size_t j = 0; j < glyphs.Length(); j++) { - wrGlyphs[j].index = glyphs[j].mIndex; - wrGlyphs[j].point = aSc.ToRelativeLayoutPoint( - LayoutDevicePoint::FromUnknownPoint(glyphs[j].mPosition)); - } - - aManager->WrBridge()->PushGlyphs(aBuilder, wrGlyphs, mFont, - wr::ToColorF(glyphArray.color().value()), - aSc, wrDestRect, wrDestRect, - !aItem->BackfaceIsHidden()); + if (bounds.IsEmpty()) { + return true; } + + RefPtr textDrawer = new TextDrawTarget(aBuilder, aSc, aManager, aItem, bounds); + RefPtr captureCtx = gfxContext::CreateOrNull(textDrawer); + PaintTextToContext(aItem->Frame(), captureCtx, aItem); + textDrawer->TerminateShadows(); + + return !textDrawer->HasUnsupportedFeatures(); } class nsDisplayBullet final : public nsDisplayItem { @@ -679,17 +675,19 @@ nsDisplayBullet::CreateWebRenderCommands(wr::DisplayListBuilder& aBuilder, mozilla::layers::WebRenderLayerManager* aManager, nsDisplayListBuilder* aDisplayListBuilder) { - ContainerLayerParameters parameter; - if (GetLayerState(aDisplayListBuilder, aManager, parameter) != LAYER_ACTIVE) { + // FIXME: avoid needing to make this target if we're drawing text + // (non-trivial refactor of all this code) + RefPtr screenRefCtx = gfxContext::CreateOrNull( + gfxPlatform::GetPlatform()->ScreenReferenceDrawTarget().get()); + Maybe br = static_cast(mFrame)-> + CreateBulletRenderer(*screenRefCtx, ToReferenceFrame()); + + if (!br) { return false; } - if (!mBulletRenderer) - return false; - - mBulletRenderer->CreateWebRenderCommands(this, aBuilder, aResources, aSc, - aManager, aDisplayListBuilder); - return true; + return br->CreateWebRenderCommands(this, aBuilder, aResources, aSc, + aManager, aDisplayListBuilder); } void nsDisplayBullet::Paint(nsDisplayListBuilder* aBuilder,