Last Comment Bug 757380 - BasicLayers Paint ignore NULL Clip when render with groupTarget
: BasicLayers Paint ignore NULL Clip when render with groupTarget
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: mozilla15
Assigned To: Oleg Romashin (:romaxa)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 780792
Blocks: 725925
  Show dependency treegraph
 
Reported: 2012-05-22 03:15 PDT by Oleg Romashin (:romaxa)
Modified: 2012-08-06 19:06 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
:bas (2.47 KB, patch)
2012-05-22 03:15 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Check clip before create group target (2.66 KB, patch)
2012-05-23 11:19 PDT, Oleg Romashin (:romaxa)
bas: review+
Details | Diff | Splinter Review
Check clip before create group target (2.74 KB, patch)
2012-05-23 11:26 PDT, Oleg Romashin (:romaxa)
romaxa: review+
Details | Diff | Splinter Review
Check clip before create group target (2.76 KB, patch)
2012-05-24 08:41 PDT, Oleg Romashin (:romaxa)
roc: review+
Details | Diff | Splinter Review
Check clip before create group target (3.05 KB, patch)
2012-05-25 15:36 PDT, Oleg Romashin (:romaxa)
bas: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2012-05-22 03:15:30 PDT
Created attachment 625955 [details] [diff] [review]
:bas

in bug 725925 I tried to avoid rendering into 1x1 surface by setting clip(0,0,0,0) to target context, and that trick does not work when we render with group target !2d case.
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicLayers.cpp#1986
groupTarget surface created even if we don't need to render anything... that is also breaking NO_COMPOSITE rendering for bug 539356.

mattwoodrow suggested to not create group target if clip is empty.
Comment 1 Bas Schouten (:bas.schouten) 2012-05-23 10:46:59 PDT
Comment on attachment 625955 [details] [diff] [review]
:bas

Review of attachment 625955 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/basic/BasicLayers.cpp
@@ +2029,5 @@
>  
>    nsRefPtr<gfxContext> groupTarget;
>    nsRefPtr<gfxASurface> untransformedSurface;
> +  gfxRect clipExtents;
> +  clipExtents = aTarget->GetClipExtents();

nit: Let's just use an initializer.

@@ +2109,1 @@
>        clipExtents = aTarget->GetClipExtents();

There shouldn't be any point in this still if we're already getting the clip extents at line 2032, right?
Comment 2 Oleg Romashin (:romaxa) 2012-05-23 11:19:40 PDT
Created attachment 626518 [details] [diff] [review]
Check clip before create group target
Comment 3 Bas Schouten (:bas.schouten) 2012-05-23 11:21:14 PDT
Comment on attachment 626518 [details] [diff] [review]
Check clip before create group target

Review of attachment 626518 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine if you remove the line that'll probably make the compile fail.

::: gfx/layers/basic/BasicLayers.cpp
@@ +1987,5 @@
>  
>    nsRefPtr<gfxContext> groupTarget;
>    nsRefPtr<gfxASurface> untransformedSurface;
> +  bool clipIsEmpty = aTarget->GetClipExtents().IsEmpty();
> +  clipExtents = aTarget->GetClipExtents();

I don't think this will compile.. clipExtents isn't declared here.
Comment 4 Oleg Romashin (:romaxa) 2012-05-23 11:26:26 PDT
Created attachment 626524 [details] [diff] [review]
Check clip before create group target

err, forgot to remove that line, sorry
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-05-23 18:04:47 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f987a378672f
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-05-23 19:52:49 PDT
http://mozillamemes.tumblr.com/post/19498220636/try-server-takes-the-beatings-so-mozilla-inbound

And backed out due to M4 crashes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/85f66ce3ebb8

https://tbpl.mozilla.org/php/getParsedLog.php?id=12009635&tree=Mozilla-Inbound

TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug93077-1.html | Exited with code 1 during test run
INFO | automation.py | Application ran for: 0:06:49.603970
INFO | automation.py | Reading PID log: /tmp/tmpaa4772pidlog
==> process 2140 launched child process 2283
INFO | automation.py | Checking for orphan process with PID: 2283
Downloading symbols from: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux-debug/1337822875/firefox-15.0a1.en-US.linux-i686.crashreporter-symbols.zip
PROCESS-CRASH | /tests/layout/base/tests/test_bug93077-1.html | application crashed (minidump found)
Crash dump filename: /tmp/tmpuRhPdH/minidumps/19cadecf-c48b-6f55-769b8c76-50b393db.dmp
Operating system: Linux
                  0.0.0 Linux 2.6.31.5-127.fc12.i686.PAE #1 SMP Sat Nov 7 21:25:57 EST 2009 i686
CPU: x86
     GenuineIntel family 6 model 23 stepping 10
     2 CPUs

Crash reason:  SIGSEGV
Crash address: 0x0

Thread 0 (crashed)
 0  libmozalloc.so!TouchBadMemory [mozalloc_abort.cpp : 35 + 0x0]
    eip = 0x004a1eee   esp = 0xbfeaa9e8   ebp = 0xbfeaaa08   ebx = 0x004a3170
    esi = 0x00c59844   edi = 0xbfeaaa38   eax = 0x0000000a   ecx = 0x004a3170
    edx = 0x00c5a32c   efl = 0x00210212
    Found by: given as instruction pointer in context
 1  libmozalloc.so!mozalloc_abort [mozalloc_abort.cpp : 56 + 0x4]
    eip = 0x004a1f2f   esp = 0xbfeaa9f0   ebp = 0xbfeaaa08   ebx = 0x004a3170
    esi = 0x00c59844   edi = 0xbfeaaa38
    Found by: call frame info
 2  libxul.so!NS_DebugBreak_P [nsDebugImpl.cpp : 382 + 0x5]
    eip = 0x020f89a9   esp = 0xbfeaaa10   ebp = 0xbfeaae48   ebx = 0x02cf55a8
    esi = 0xbfeaae24   edi = 0xbfeaaa38
    Found by: call frame info
 3  libxul.so!mozilla::layers::BasicLayerManager::PaintLayer [BasicLayers.cpp : 2050 + 0x23]
    eip = 0x021818d2   esp = 0xbfeaae50   ebp = 0xbfeab2f8   ebx = 0x02cf55a8
    esi = 0x00000001   edi = 0xbfeab000
    Found by: call frame info
 4  libxul.so!mozilla::layers::BasicLayerManager::PaintLayer [BasicLayers.cpp : 2039 + 0x23]
    eip = 0x02181845   esp = 0xbfeab300   ebp = 0xbfeab7a8   ebx = 0x02cf55a8
    esi = 0x00000001   edi = 0xbfeab4b0
    Found by: call frame info
 5  libxul.so!mozilla::layers::BasicLayerManager::PaintLayer [BasicLayers.cpp : 2039 + 0x23]
    eip = 0x02181845   esp = 0xbfeab7b0   ebp = 0xbfeabc58   ebx = 0x02cf55a8
    esi = 0x00000001   edi = 0xbfeab960
    Found by: call frame info
 6  libxul.so!mozilla::layers::BasicLayerManager::EndTransactionInternal [BasicLayers.cpp : 1688 + 0x18]
    eip = 0x02182962   esp = 0xbfeabc60   ebp = 0xbfeabe38   ebx = 0x02cf55a8
    esi = 0x09ada53c   edi = 0xbfeabe10
    Found by: call frame info
 7  libxul.so!mozilla::layers::BasicShadowLayerManager::EndTransaction [BasicLayers.cpp : 3527 + 0xe]
    eip = 0x02182a99   esp = 0xbfeabe40   ebp = 0xbfeabe58   ebx = 0x02cf55a8
    esi = 0x09ada508   edi = 0x0ad944f0
    Found by: call frame info
 8  libxul.so!nsDisplayList::PaintForFrame [nsDisplayList.cpp : 646 + 0x11]
    eip = 0x013d3f43   esp = 0xbfeabe60   ebp = 0xbfeabf68   ebx = 0x02cf55a8
    esi = 0xbfeabf48   edi = 0x0ad944f0
    Found by: call frame info
 9  libxul.so!nsDisplayList::PaintRoot [nsDisplayList.cpp : 551 + 0x16]
    eip = 0x013d4019   esp = 0xbfeabf70   ebp = 0xbfeabfb8   ebx = 0x02cf55a8
Comment 7 Oleg Romashin (:romaxa) 2012-05-24 08:41:06 PDT
Created attachment 626824 [details] [diff] [review]
Check clip before create group target
Comment 9 Phil Ringnalda (:philor) 2012-05-24 19:58:01 PDT
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f2ec7e8407e5 - either this or you patch for bug 757933 was hitting mochitest-4 assertions like https://tbpl.mozilla.org/php/getParsedLog.php?id=12048582&tree=Mozilla-Inbound&full=1
Comment 10 Oleg Romashin (:romaxa) 2012-05-25 14:07:39 PDT
Ok, runtime abort must be moved below clip-extents check.
Need finish try build and add new version.
https://tbpl.mozilla.org/?tree=Try&rev=c30e13ce681c
Comment 11 Oleg Romashin (:romaxa) 2012-05-25 15:36:02 PDT
Created attachment 627393 [details] [diff] [review]
Check clip before create group target

ok, one more try, this should be fine
Comment 13 Ed Morley [:emorley] 2012-05-29 10:21:54 PDT
https://hg.mozilla.org/mozilla-central/rev/e6c4152034ed

Note You need to log in before you can comment on or make changes to this bug.