Skip to content

Commit 63f2fca

Browse files
chethaaseAndroid (Google) Code Review
authored andcommitted
Merge "Fix bug with Fbo layer clipping" into jb-mr1-dev
2 parents 428f539 + d48885a commit 63f2fca

File tree

2 files changed

+54
-58
lines changed

2 files changed

+54
-58
lines changed

libs/hwui/OpenGLRenderer.cpp

Lines changed: 52 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ int OpenGLRenderer::saveLayer(float left, float top, float right, float bottom,
440440
mode = SkXfermode::kSrcOver_Mode;
441441
}
442442

443-
createLayer(mSnapshot, left, top, right, bottom, alpha, mode, flags, previousFbo);
443+
createLayer(left, top, right, bottom, alpha, mode, flags, previousFbo);
444444
}
445445

446446
return count;
@@ -508,44 +508,56 @@ int OpenGLRenderer::saveLayerAlpha(float left, float top, float right, float bot
508508
* buffer is left untouched until the first drawing operation. Only when
509509
* something actually gets drawn are the layers regions cleared.
510510
*/
511-
bool OpenGLRenderer::createLayer(sp<Snapshot> snapshot, float left, float top,
512-
float right, float bottom, int alpha, SkXfermode::Mode mode,
513-
int flags, GLuint previousFbo) {
511+
bool OpenGLRenderer::createLayer(float left, float top, float right, float bottom,
512+
int alpha, SkXfermode::Mode mode, int flags, GLuint previousFbo) {
514513
LAYER_LOGD("Requesting layer %.2fx%.2f", right - left, bottom - top);
515514
LAYER_LOGD("Layer cache size = %d", mCaches.layerCache.getSize());
516515

517516
const bool fboLayer = flags & SkCanvas::kClipToLayer_SaveFlag;
518517

519518
// Window coordinates of the layer
519+
Rect clip;
520520
Rect bounds(left, top, right, bottom);
521-
if (!fboLayer) {
522-
mSnapshot->transform->mapRect(bounds);
523-
524-
// Layers only make sense if they are in the framebuffer's bounds
525-
if (bounds.intersect(*snapshot->clipRect)) {
526-
// We cannot work with sub-pixels in this case
527-
bounds.snapToPixelBoundaries();
528-
529-
// When the layer is not an FBO, we may use glCopyTexImage so we
530-
// need to make sure the layer does not extend outside the bounds
531-
// of the framebuffer
532-
if (!bounds.intersect(snapshot->previous->viewport)) {
533-
bounds.setEmpty();
534-
}
535-
} else {
521+
Rect untransformedBounds(bounds);
522+
mSnapshot->transform->mapRect(bounds);
523+
524+
// Layers only make sense if they are in the framebuffer's bounds
525+
if (bounds.intersect(*mSnapshot->clipRect)) {
526+
// We cannot work with sub-pixels in this case
527+
bounds.snapToPixelBoundaries();
528+
529+
// When the layer is not an FBO, we may use glCopyTexImage so we
530+
// need to make sure the layer does not extend outside the bounds
531+
// of the framebuffer
532+
if (!bounds.intersect(mSnapshot->previous->viewport)) {
536533
bounds.setEmpty();
534+
} else if (fboLayer) {
535+
clip.set(bounds);
536+
mat4 inverse;
537+
inverse.loadInverse(*mSnapshot->transform);
538+
inverse.mapRect(clip);
539+
clip.snapToPixelBoundaries();
540+
if (clip.intersect(untransformedBounds)) {
541+
clip.translate(-left, -top);
542+
bounds.set(untransformedBounds);
543+
} else {
544+
clip.setEmpty();
545+
}
537546
}
547+
} else {
548+
bounds.setEmpty();
538549
}
539550

540551
if (bounds.isEmpty() || bounds.getWidth() > mCaches.maxTextureSize ||
541-
bounds.getHeight() > mCaches.maxTextureSize) {
542-
snapshot->empty = fboLayer;
552+
bounds.getHeight() > mCaches.maxTextureSize ||
553+
(fboLayer && clip.isEmpty())) {
554+
mSnapshot->empty = fboLayer;
543555
} else {
544-
snapshot->invisible = snapshot->invisible || (alpha <= ALPHA_THRESHOLD && fboLayer);
556+
mSnapshot->invisible = mSnapshot->invisible || (alpha <= ALPHA_THRESHOLD && fboLayer);
545557
}
546558

547559
// Bail out if we won't draw in this snapshot
548-
if (snapshot->invisible || snapshot->empty) {
560+
if (mSnapshot->invisible || mSnapshot->empty) {
549561
return false;
550562
}
551563

@@ -563,23 +575,23 @@ bool OpenGLRenderer::createLayer(sp<Snapshot> snapshot, float left, float top,
563575
layer->setBlend(true);
564576

565577
// Save the layer in the snapshot
566-
snapshot->flags |= Snapshot::kFlagIsLayer;
567-
snapshot->layer = layer;
578+
mSnapshot->flags |= Snapshot::kFlagIsLayer;
579+
mSnapshot->layer = layer;
568580

569581
if (fboLayer) {
570-
return createFboLayer(layer, bounds, snapshot, previousFbo);
582+
return createFboLayer(layer, bounds, clip, previousFbo);
571583
} else {
572584
// Copy the framebuffer into the layer
573585
layer->bindTexture();
574586
if (!bounds.isEmpty()) {
575587
if (layer->isEmpty()) {
576588
glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA,
577-
bounds.left, snapshot->height - bounds.bottom,
589+
bounds.left, mSnapshot->height - bounds.bottom,
578590
layer->getWidth(), layer->getHeight(), 0);
579591
layer->setEmpty(false);
580592
} else {
581593
glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, bounds.left,
582-
snapshot->height - bounds.bottom, bounds.getWidth(), bounds.getHeight());
594+
mSnapshot->height - bounds.bottom, bounds.getWidth(), bounds.getHeight());
583595
}
584596

585597
// Enqueue the buffer coordinates to clear the corresponding region later
@@ -590,35 +602,20 @@ bool OpenGLRenderer::createLayer(sp<Snapshot> snapshot, float left, float top,
590602
return true;
591603
}
592604

593-
bool OpenGLRenderer::createFboLayer(Layer* layer, Rect& bounds, sp<Snapshot> snapshot,
594-
GLuint previousFbo) {
605+
bool OpenGLRenderer::createFboLayer(Layer* layer, Rect& bounds, Rect& clip, GLuint previousFbo) {
595606
layer->setFbo(mCaches.fboCache.get());
596607

597-
snapshot->region = &snapshot->layer->region;
598-
snapshot->flags |= Snapshot::kFlagFboTarget;
599-
600-
Rect clip(bounds);
601-
snapshot->transform->mapRect(clip);
602-
clip.intersect(*snapshot->clipRect);
603-
clip.snapToPixelBoundaries();
604-
clip.intersect(snapshot->previous->viewport);
605-
606-
mat4 inverse;
607-
inverse.loadInverse(*mSnapshot->transform);
608-
609-
inverse.mapRect(clip);
610-
clip.snapToPixelBoundaries();
611-
clip.intersect(bounds);
612-
clip.translate(-bounds.left, -bounds.top);
613-
614-
snapshot->flags |= Snapshot::kFlagIsFboLayer;
615-
snapshot->fbo = layer->getFbo();
616-
snapshot->resetTransform(-bounds.left, -bounds.top, 0.0f);
617-
snapshot->resetClip(clip.left, clip.top, clip.right, clip.bottom);
618-
snapshot->viewport.set(0.0f, 0.0f, bounds.getWidth(), bounds.getHeight());
619-
snapshot->height = bounds.getHeight();
620-
snapshot->flags |= Snapshot::kFlagDirtyOrtho;
621-
snapshot->orthoMatrix.load(mOrthoMatrix);
608+
mSnapshot->region = &mSnapshot->layer->region;
609+
mSnapshot->flags |= Snapshot::kFlagFboTarget;
610+
611+
mSnapshot->flags |= Snapshot::kFlagIsFboLayer;
612+
mSnapshot->fbo = layer->getFbo();
613+
mSnapshot->resetTransform(-bounds.left, -bounds.top, 0.0f);
614+
mSnapshot->resetClip(clip.left, clip.top, clip.right, clip.bottom);
615+
mSnapshot->viewport.set(0.0f, 0.0f, bounds.getWidth(), bounds.getHeight());
616+
mSnapshot->height = bounds.getHeight();
617+
mSnapshot->flags |= Snapshot::kFlagDirtyOrtho;
618+
mSnapshot->orthoMatrix.load(mOrthoMatrix);
622619

623620
// Bind texture to FBO
624621
glBindFramebuffer(GL_FRAMEBUFFER, layer->getFbo());

libs/hwui/OpenGLRenderer.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ class OpenGLRenderer {
380380
*
381381
* @return True if the layer was successfully created, false otherwise
382382
*/
383-
bool createLayer(sp<Snapshot> snapshot, float left, float top, float right, float bottom,
383+
bool createLayer(float left, float top, float right, float bottom,
384384
int alpha, SkXfermode::Mode mode, int flags, GLuint previousFbo);
385385

386386
/**
@@ -391,8 +391,7 @@ class OpenGLRenderer {
391391
* @param bounds The bounds of the layer
392392
* @param previousFbo The name of the current framebuffer
393393
*/
394-
bool createFboLayer(Layer* layer, Rect& bounds, sp<Snapshot> snapshot,
395-
GLuint previousFbo);
394+
bool createFboLayer(Layer* layer, Rect& bounds, Rect& clip, GLuint previousFbo);
396395

397396
/**
398397
* Compose the specified layer as a region.

0 commit comments

Comments
 (0)