Bug 1101685 - Optionally assert in loggers, default to true on gfxCriticalError. Clean up the calls where large texture sizes were triggering the asserts in tests. r=nical
☠☠ backed out by a82581055c06 ☠ ☠
authorMilan Sreckovic <milan@mozilla.com>
Tue, 16 Dec 2014 13:22:26 -0500
changeset 220180 8b751f12a3ad6154af32c98a8264310f9dd15222
parent 220097 d3ce465f852c5d82fbc4df8168b9de48b9bf189c
child 220181 7fe7d8036eac311beb8aa600a158e7fa69e5431e
push id10457
push userryanvm@gmail.com
push dateThu, 18 Dec 2014 01:54:25 +0000
treeherderfx-team@0e441ff66c5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1101685
milestone37.0a1
Bug 1101685 - Optionally assert in loggers, default to true on gfxCriticalError. Clean up the calls where large texture sizes were triggering the asserts in tests. r=nical
gfx/2d/2D.h
gfx/2d/DrawTargetCairo.cpp
gfx/2d/DrawTargetD2D.cpp
gfx/2d/DrawTargetD2D1.cpp
gfx/2d/Factory.cpp
gfx/2d/Logging.h
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -1070,16 +1070,21 @@ public:
   static bool HasSSE2();
 
   /** Make sure that the given dimensions don't overflow a 32-bit signed int
    * using 4 bytes per pixel; optionally, make sure that either dimension
    * doesn't exceed the given limit.
    */
   static bool CheckSurfaceSize(const IntSize &sz, int32_t limit = 0);
 
+  /** Make sure the given dimension satisfies the CheckSurfaceSize and is
+   * within 8k limit.  The 8k value is chosen a bit randomly.
+   */
+  static bool ReasonableSurfaceSize(const IntSize &aSize);
+
   static TemporaryRef<DrawTarget> CreateDrawTargetForCairoSurface(cairo_surface_t* aSurface, const IntSize& aSize, SurfaceFormat* aFormat = nullptr);
 
   static TemporaryRef<DrawTarget>
     CreateDrawTarget(BackendType aBackend, const IntSize &aSize, SurfaceFormat aFormat);
 
   static TemporaryRef<DrawTarget>
     CreateRecordingDrawTarget(DrawEventRecorder *aRecorder, DrawTarget *aDT);
      
--- a/gfx/2d/DrawTargetCairo.cpp
+++ b/gfx/2d/DrawTargetCairo.cpp
@@ -1480,17 +1480,17 @@ DrawTargetCairo::CreateSimilarDrawTarget
 
   if (!cairo_surface_status(similar)) {
     RefPtr<DrawTargetCairo> target = new DrawTargetCairo();
     if (target->InitAlreadyReferenced(similar, aSize)) {
       return target.forget();
     }
   }
 
-  gfxCriticalError() << "Failed to create similar cairo surface! Size: " << aSize << " Status: " << cairo_surface_status(similar);
+  gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(aSize))) << "Failed to create similar cairo surface! Size: " << aSize << " Status: " << cairo_surface_status(similar);
 
   return nullptr;
 }
 
 bool
 DrawTargetCairo::InitAlreadyReferenced(cairo_surface_t* aSurface, const IntSize& aSize, SurfaceFormat* aFormat)
 {
   if (cairo_surface_status(aSurface)) {
--- a/gfx/2d/DrawTargetD2D.cpp
+++ b/gfx/2d/DrawTargetD2D.cpp
@@ -1356,17 +1356,17 @@ DrawTargetD2D::Init(const IntSize &aSize
                              mSize.width,
                              mSize.height,
                              1, 1);
   desc.BindFlags = D3D10_BIND_RENDER_TARGET | D3D10_BIND_SHADER_RESOURCE;
 
   hr = mDevice->CreateTexture2D(&desc, nullptr, byRef(mTexture));
 
   if (FAILED(hr)) {
-    gfxCriticalError() << "Failed to init Direct2D DrawTarget. Size: " << mSize << " Code: " << hexa(hr);
+    gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(aSize))) << "Failed to init Direct2D DrawTarget. Size: " << mSize << " Code: " << hexa(hr);
     return false;
   }
 
   if (!InitD2DRenderTarget()) {
     return false;
   }
 
   mRT->Clear(D2D1::ColorF(0, 0));
--- a/gfx/2d/DrawTargetD2D1.cpp
+++ b/gfx/2d/DrawTargetD2D1.cpp
@@ -648,17 +648,17 @@ DrawTargetD2D1::CreateSourceSurfaceFromD
 {
   RefPtr<ID2D1Bitmap1> bitmap;
 
   HRESULT hr = mDC->CreateBitmap(D2DIntSize(aSize), aData, aStride,
                                  D2D1::BitmapProperties1(D2D1_BITMAP_OPTIONS_NONE, D2DPixelFormat(aFormat)),
                                  byRef(bitmap));
 
   if (FAILED(hr) || !bitmap) {
-    gfxCriticalError() << "[D2D1.1] CreateBitmap failure " << aSize << " Code: " << hexa(hr);
+    gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(aSize))) << "[D2D1.1] 1CreateBitmap failure " << aSize << " Code: " << hexa(hr);
     return nullptr;
   }
 
   return new SourceSurfaceD2D1(bitmap.get(), mDC, aFormat, aSize);
 }
 
 TemporaryRef<DrawTarget>
 DrawTargetD2D1::CreateSimilarDrawTarget(const IntSize &aSize, SurfaceFormat aFormat) const
@@ -737,17 +737,17 @@ DrawTargetD2D1::CreateFilter(FilterType 
 bool
 DrawTargetD2D1::Init(ID3D11Texture2D* aTexture, SurfaceFormat aFormat)
 {
   HRESULT hr;
 
   hr = Factory::GetD2D1Device()->CreateDeviceContext(D2D1_DEVICE_CONTEXT_OPTIONS_ENABLE_MULTITHREADED_OPTIMIZATIONS, byRef(mDC));
 
   if (FAILED(hr)) {
-    gfxCriticalError() <<"[D2D1.1] Failed to create a DeviceContext, code: " << hexa(hr);
+    gfxCriticalError() <<"[D2D1.1] 1Failed to create a DeviceContext, code: " << hexa(hr);
     return false;
   }
 
   RefPtr<IDXGISurface> dxgiSurface;
   aTexture->QueryInterface(__uuidof(IDXGISurface),
                            (void**)((IDXGISurface**)byRef(dxgiSurface)));
   if (!dxgiSurface) {
     gfxCriticalError() <<"[D2D1.1] Failed to obtain a DXGI surface.";
@@ -774,17 +774,17 @@ DrawTargetD2D1::Init(ID3D11Texture2D* aT
   mSize.width = desc.Width;
   mSize.height = desc.Height;
   props.pixelFormat.alphaMode = D2D1_ALPHA_MODE_PREMULTIPLIED;
   props.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM;
 
   hr = mDC->CreateBitmap(D2DIntSize(mSize), nullptr, 0, props, (ID2D1Bitmap1**)byRef(mTempBitmap));
 
   if (FAILED(hr)) {
-    gfxCriticalError() << "[D2D1.1] CreateBitmap failure " << mSize << " Code: " << hexa(hr);
+    gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(mSize))) << "[D2D1.1] 2CreateBitmap failure " << mSize << " Code: " << hexa(hr);
     return false;
   }
 
   mDC->SetTarget(mBitmap);
 
   mDC->BeginDraw();
   return true;
 }
@@ -792,37 +792,37 @@ DrawTargetD2D1::Init(ID3D11Texture2D* aT
 bool
 DrawTargetD2D1::Init(const IntSize &aSize, SurfaceFormat aFormat)
 {
   HRESULT hr;
 
   hr = Factory::GetD2D1Device()->CreateDeviceContext(D2D1_DEVICE_CONTEXT_OPTIONS_ENABLE_MULTITHREADED_OPTIMIZATIONS, byRef(mDC));
 
   if (FAILED(hr)) {
-    gfxCriticalError() <<"[D2D1.1] Failed to create a DeviceContext, code: " << hexa(hr);
+    gfxCriticalError() <<"[D2D1.1] 2Failed to create a DeviceContext, code: " << hexa(hr);
     return false;
   }
 
   if (mDC->GetMaximumBitmapSize() < UINT32(aSize.width) ||
       mDC->GetMaximumBitmapSize() < UINT32(aSize.height)) {
-    // This is 'ok'
-    gfxCriticalError() << "[D2D1.1] Attempt to use unsupported surface size " << aSize;
+    // This is 'ok', so don't assert
+    gfxCriticalError(CriticalLog::DefaultOptions(false)) << "[D2D1.1] Attempt to use unsupported surface size " << aSize;
     return false;
   }
 
   D2D1_BITMAP_PROPERTIES1 props;
   props.dpiX = 96;
   props.dpiY = 96;
   props.pixelFormat = D2DPixelFormat(aFormat);
   props.colorContext = nullptr;
   props.bitmapOptions = D2D1_BITMAP_OPTIONS_TARGET;
   hr = mDC->CreateBitmap(D2DIntSize(aSize), nullptr, 0, props, (ID2D1Bitmap1**)byRef(mBitmap));
 
   if (FAILED(hr)) {
-    gfxCriticalError() << "[D2D1.1] CreateBitmap failure " << aSize << " Code: " << hexa(hr);
+    gfxCriticalError() << "[D2D1.1] 3CreateBitmap failure " << aSize << " Code: " << hexa(hr);
     return false;
   }
 
   props.pixelFormat.alphaMode = D2D1_ALPHA_MODE_PREMULTIPLIED;
   props.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM;
 
   hr = mDC->CreateBitmap(D2DIntSize(aSize), nullptr, 0, props, (ID2D1Bitmap1**)byRef(mTempBitmap));
 
@@ -1340,17 +1340,17 @@ DrawTargetD2D1::OptimizeSourceSurface(So
   }
 
   RefPtr<ID2D1Bitmap1> bitmap;
   HRESULT hr = mDC->CreateBitmap(D2DIntSize(data->GetSize()), map.mData, map.mStride,
                                  D2D1::BitmapProperties1(D2D1_BITMAP_OPTIONS_NONE, D2DPixelFormat(data->GetFormat())),
                                  byRef(bitmap));
 
   if (FAILED(hr)) {
-    gfxCriticalError() << "[D2D1.1] CreateBitmap failure " << data->GetSize() << " Code: " << hexa(hr);
+    gfxCriticalError(CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(data->GetSize()))) << "[D2D1.1] 4CreateBitmap failure " << data->GetSize() << " Code: " << hexa(hr);
   }
 
   data->Unmap();
 
   if (!bitmap) {
     return data.forget();
   }
 
--- a/gfx/2d/Factory.cpp
+++ b/gfx/2d/Factory.cpp
@@ -211,21 +211,34 @@ Factory::HasSSE2()
     sDetectionState = HasCPUIDBit(1u, edx, (1u<<26)) ? HAS_SSE2 : NO_SSE2;
   }
   return sDetectionState == HAS_SSE2;
 #else
   return false;
 #endif
 }
 
+// If the size is "reasonable", we want gfxCriticalError to assert, so
+// this is the option set up for it.
+inline int LoggerOptionsBasedOnSize(const IntSize& aSize)
+{
+  return CriticalLog::DefaultOptions(Factory::ReasonableSurfaceSize(aSize));
+}
+
+bool
+Factory::ReasonableSurfaceSize(const IntSize &aSize)
+{
+  return Factory::CheckSurfaceSize(aSize,8192);
+}
+
 bool
 Factory::CheckSurfaceSize(const IntSize &sz, int32_t limit)
 {
-  if (sz.width < 0 || sz.height < 0) {
-    gfxDebug() << "Surface width or height < 0!";
+  if (sz.width <= 0 || sz.height <= 0) {
+    gfxDebug() << "Surface width or height <= 0!";
     return false;
   }
 
   // reject images with sides bigger than limit
   if (limit && (sz.width > limit || sz.height > limit)) {
     gfxDebug() << "Surface size too large (exceeds caller's limit)!";
     return false;
   }
@@ -260,17 +273,17 @@ Factory::CheckSurfaceSize(const IntSize 
 
   return true;
 }
 
 TemporaryRef<DrawTarget>
 Factory::CreateDrawTarget(BackendType aBackend, const IntSize &aSize, SurfaceFormat aFormat)
 {
   if (!CheckSurfaceSize(aSize)) {
-    gfxCriticalError() << "Failed to allocate a surface due to invalid size " << aSize;
+    gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to allocate a surface due to invalid size " << aSize;
     return nullptr;
   }
 
   RefPtr<DrawTarget> retVal;
   switch (aBackend) {
 #ifdef WIN32
   case BackendType::DIRECT2D:
     {
@@ -332,38 +345,38 @@ Factory::CreateDrawTarget(BackendType aB
   }
 
   if (mRecorder && retVal) {
     return new DrawTargetRecording(mRecorder, retVal);
   }
 
   if (!retVal) {
     // Failed
-    gfxCriticalError() << "Failed to create DrawTarget, Type: " << int(aBackend) << " Size: " << aSize;
+    gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to create DrawTarget, Type: " << int(aBackend) << " Size: " << aSize;
   }
-  
+
   return retVal.forget();
 }
 
 TemporaryRef<DrawTarget>
 Factory::CreateRecordingDrawTarget(DrawEventRecorder *aRecorder, DrawTarget *aDT)
 {
   return new DrawTargetRecording(aRecorder, aDT);
 }
 
 TemporaryRef<DrawTarget>
-Factory::CreateDrawTargetForData(BackendType aBackend, 
-                                 unsigned char *aData, 
-                                 const IntSize &aSize, 
-                                 int32_t aStride, 
+Factory::CreateDrawTargetForData(BackendType aBackend,
+                                 unsigned char *aData,
+                                 const IntSize &aSize,
+                                 int32_t aStride,
                                  SurfaceFormat aFormat)
 {
   MOZ_ASSERT(aData);
   if (!CheckSurfaceSize(aSize)) {
-    gfxCriticalError() << "Failed to allocate a surface due to invalid size " << aSize;
+    gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to allocate a surface due to invalid size " << aSize;
     return nullptr;
   }
 
   RefPtr<DrawTarget> retVal;
 
   switch (aBackend) {
 #ifdef USE_SKIA
   case BackendType::SKIA:
@@ -781,17 +794,17 @@ Factory::CreateWrappingDataSourceSurface
 }
 
 TemporaryRef<DataSourceSurface>
 Factory::CreateDataSourceSurface(const IntSize &aSize,
                                  SurfaceFormat aFormat,
                                  bool aZero)
 {
   if (!CheckSurfaceSize(aSize)) {
-    gfxCriticalError() << "Failed to allocate a surface due to invalid size " << aSize;
+    gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "Failed to allocate a surface due to invalid size " << aSize;
     return nullptr;
   }
 
   RefPtr<SourceSurfaceAlignedRawData> newSurf = new SourceSurfaceAlignedRawData();
   if (newSurf->Init(aSize, aFormat, aZero)) {
     return newSurf.forget();
   }
 
@@ -801,26 +814,26 @@ Factory::CreateDataSourceSurface(const I
 
 TemporaryRef<DataSourceSurface>
 Factory::CreateDataSourceSurfaceWithStride(const IntSize &aSize,
                                            SurfaceFormat aFormat,
                                            int32_t aStride,
                                            bool aZero)
 {
   if (aStride < aSize.width * BytesPerPixel(aFormat)) {
-    gfxCriticalError() << "CreateDataSourceSurfaceWithStride failed with bad stride";
+    gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "CreateDataSourceSurfaceWithStride failed with bad stride " << aStride << ", " << aSize << ", " << aFormat;
     return nullptr;
   }
 
   RefPtr<SourceSurfaceAlignedRawData> newSurf = new SourceSurfaceAlignedRawData();
   if (newSurf->InitWithStride(aSize, aFormat, aStride, aZero)) {
     return newSurf.forget();
   }
 
-  gfxCriticalError() << "CreateDataSourceSurfaceWithStride failed to initialize";
+  gfxCriticalError(LoggerOptionsBasedOnSize(aSize)) << "CreateDataSourceSurfaceWithStride failed to initialize " << aSize << ", " << aFormat << ", " << aStride << ", " << aZero;
   return nullptr;
 }
 
 TemporaryRef<DrawEventRecorder>
 Factory::CreateEventRecorderForFile(const char *aFilename)
 {
   return new DrawEventRecorderFile(aFilename);
 }
--- a/gfx/2d/Logging.h
+++ b/gfx/2d/Logging.h
@@ -107,16 +107,44 @@ private:
     // The default values (last parameter) should match the initialization
     // values in Factory.cpp, otherwise the standalone Moz2D will get different
     // defaults.
     sAccess->LivePref("gfx.logging.level", &sGfxLogLevel, LOG_DEFAULT);
   }
   static PreferenceAccess* sAccess;
 };
 
+/// Graphics logging is available in both debug and release builds and is
+/// controlled with a gfx.logging.level preference. If not set, the default
+/// for the preference is 5 in the debug builds, 1 in the release builds.
+///
+/// gfxDebug only works in the debug builds, and is used for information
+/// level messages, helping with debugging.  In addition to only working
+/// in the debug builds, the value of the above preference of 3 or higher
+/// is required.
+///
+/// gfxWarning messages are available in both debug and release builds,
+/// on by default in the debug builds, and off by default in the release builds.
+/// Setting the preference gfx.logging.level to a value of 2 or higher will
+/// show the warnings.
+///
+/// gfxCriticalError is available in debug and release builds by default.
+/// It is only unavailable if gfx.logging.level is set to 0 (or less.)
+/// It outputs the message to stderr or equivalent, like gfxWarning.
+/// In the event of a crash, the crash report is annotated with first and
+/// the last few of these errors, under the key GraphicsCriticalError.
+/// The total number of errors stored in the crash report is controlled
+/// by preference gfx.logging.crash.length (default is six, so by default,
+/// the first as well as the last five would show up in the crash log.)
+///
+/// On platforms that support PR_LOGGING, the story is slightly more involved.
+/// In that case, unless gfx.logging.level is set to 4 or higher, the output
+/// is further controlled by "gfx2d" PR logging module.  However, in the case
+/// where such module would disable the output, in all but gfxDebug cases,
+/// we will still send a printf.
 struct BasicLogger
 {
   // For efficiency, this method exists and copies the logic of the
   // OutputMessage below.  If making any changes here, also make it
   // in the appropriate places in that method.
   static bool ShouldOutputMessage(int aLevel) {
     if (PreferenceAccess::sGfxLogLevel >= aLevel) {
 #if defined(WIN32) && !defined(PR_LOGGING)
@@ -187,32 +215,44 @@ public:
   ~NoLog() {}
 
   template<typename T>
   NoLog &operator <<(const T &aLogText) { return *this; }
 };
 
 MOZ_BEGIN_ENUM_CLASS(LogOptions, int)
   NoNewline = 0x01,
-  AutoPrefix = 0x02
+  AutoPrefix = 0x02,
+  AssertOnCall = 0x04
 MOZ_END_ENUM_CLASS(LogOptions)
 
 template<typename T>
 struct Hexa {
   explicit Hexa(T aVal) : mVal(aVal) {}
   T mVal;
 };
 template<typename T>
 Hexa<T> hexa(T val) { return Hexa<T>(val); }
 
 template<int L, typename Logger = BasicLogger>
 class Log
 {
 public:
-  explicit Log(int aOptions = (int)LogOptions::AutoPrefix)
+  // The default is to have the prefix, have the new line, and for critical
+  // logs assert on each call.
+  static int DefaultOptions(bool aWithAssert = true) {
+    return (int(LogOptions::AutoPrefix) |
+            (aWithAssert ? int(LogOptions::AssertOnCall) : 0));
+  }
+
+  // Note that we're calling BasicLogger::ShouldOutputMessage, rather than
+  // Logger::ShouldOutputMessage.  Since we currently don't have a different
+  // version of that method for different loggers, this is OK. Once we do,
+  // change BasicLogger::ShouldOutputMessage to Logger::ShouldOutputMessage.
+  explicit Log(int aOptions = Log::DefaultOptions(L == LOG_CRITICAL))
     : mOptions(aOptions)
     , mLogIt(BasicLogger::ShouldOutputMessage(L))
   {
     if (mLogIt && AutoPrefix()) {
       mMessage << "[GFX" << L << "]: ";
     }
   }
   ~Log() {
@@ -428,16 +468,19 @@ public:
   inline bool NoNewline() const { return mOptions & int(LogOptions::NoNewline); }
   inline bool AutoPrefix() const { return mOptions & int(LogOptions::AutoPrefix); }
 
 
 private:
   void WriteLog(const std::string &aString) {
     if (MOZ_UNLIKELY(LogIt())) {
       Logger::OutputMessage(aString, L, NoNewline());
+      if (mOptions & int(LogOptions::AssertOnCall)) {
+        MOZ_ASSERT(false, "An assert from the graphics logger");
+      }
     }
   }
 
   std::stringstream mMessage;
   int mOptions;
   bool mLogIt;
 };