Prevent crash in wxD2DRenderer::CreateContext* if BindDC() call fails. (PR #26052)

David Hansel

unread,

Dec 23, 2025, 3:23:03 PM (3 days ago) Dec 23

to wx-...@googlegroups.com, Subscribed

Instead return NULL and let user code handle the error situation.
This fixes #26041

You can view, comment on, or merge this pull request online at:

https://github.com/wxWidgets/wxWidgets/pull/26052

Commit Summary

bd789bf Prevent crash in wxD2DRenderer::CreateContext* if BindDC() call fails.

File Changes

(1 file)

M src/msw/graphicsd2d.cpp (105)

Patch Links:

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

VZ

unread,

Dec 23, 2025, 3:37:51 PM (3 days ago) Dec 23

to wx-...@googlegroups.com, Subscribed

@vadz commented on this pull request.

Thanks, this looks like a good change, but it would be nice to polish it a bit, please do it if you can, otherwise I'll try to do later (next year).

In src/msw/graphicsd2d.cpp:

>  
-        m_nativeResource = renderTarget;
+        // BindDC may fail with E_INVALIDARG if the given RECT is too large.
+        // It is better to let the user code handle this condition than

The trouble is that currently there is no way for the application code to even know that an error occurred... Could we propagate the error from here somehow?

In src/msw/graphicsd2d.cpp:

> @@ -757,7 +757,12 @@ class wxD2DResourceHolder: public wxManagedResourceHolder
 
         DoAcquireResource();
 
-        wxCHECK_RESOURCE_HOLDER_POST();
+        // do NOT use wxCHECK_RESOURCE_HOLDER_POST

This macro (and then this comment) should be just removed entirely as it's not used anywhere else.

In src/msw/graphicsd2d.cpp:

>      // Remove all layers from the stack of layers.
     while ( !m_layers.empty() )
     {
         LayerData ld = m_layers.top();
         m_layers.pop();
 
-        GetRenderTarget()->PopLayer();
+        if (target != nullptr) target->PopLayer();

Minor, but in wx coding style this should be written as

⬇️ Suggested change

-        if (target != nullptr) target->PopLayer();
+        if ( target != nullptr )
+            target->PopLayer();

(here and below, of course)

In src/msw/graphicsd2d.cpp:

> @@ -5169,7 +5189,13 @@ wxD2DRenderer::~wxD2DRenderer()
 
 wxGraphicsContext* wxD2DRenderer::CreateContext(const wxWindowDC& dc)
 {
-    return new wxD2DContext(this, m_direct2dFactory, dc);
+    wxD2DContext* d2d = new wxD2DContext(this, m_direct2dFactory, dc);

It would be nice to make a factory function to be able to write just

return wxD2DContext::New(this, m_direct2dFactory, dc);

here and below instead of repeating the same code in multiple places.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

David Hansel

unread,

Dec 23, 2025, 4:08:22 PM (3 days ago) Dec 23

to wx-...@googlegroups.com, Subscribed

@dhansel commented on this pull request.

In src/msw/graphicsd2d.cpp:

>  
-        m_nativeResource = renderTarget;
+        // BindDC may fail with E_INVALIDARG if the given RECT is too large.
+        // It is better to let the user code handle this condition than

My idea was that the wxD2DRenderer::CreateContext() function (which is called from application code) will now return NULL instead of crashing - so the application can see there was a problem. Am I misunderstanding/missing something?

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

David Hansel

unread,

Dec 23, 2025, 4:11:49 PM (3 days ago) Dec 23

to wx-...@googlegroups.com, Subscribed

@dhansel commented on this pull request.

In src/msw/graphicsd2d.cpp:

> @@ -5169,7 +5189,13 @@ wxD2DRenderer::~wxD2DRenderer()
 
 wxGraphicsContext* wxD2DRenderer::CreateContext(const wxWindowDC& dc)
 {
-    return new wxD2DContext(this, m_direct2dFactory, dc);
+    wxD2DContext* d2d = new wxD2DContext(this, m_direct2dFactory, dc);

Sorry, not quite understanding this. The "dc" in the different wxD2DContext() constructor calls is a different type every time. So wouldn't that mean I would need a wxD2DContext::New() function for each? Maybe you can point me to a similar construct in existing code so I can take a closer look?

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Dec 23, 2025, 5:17:04 PM (3 days ago) Dec 23

to wx-...@googlegroups.com, Subscribed

@vadz commented on this pull request.

In src/msw/graphicsd2d.cpp:

> @@ -5169,7 +5189,13 @@ wxD2DRenderer::~wxD2DRenderer()
 
 wxGraphicsContext* wxD2DRenderer::CreateContext(const wxWindowDC& dc)
 {
-    return new wxD2DContext(this, m_direct2dFactory, dc);
+    wxD2DContext* d2d = new wxD2DContext(this, m_direct2dFactory, dc);

Sorry, I don't have an example at hand but we could make a template New(), forwarding the arguments to the appropriate ctor (which would become private).

Alternatively, and maybe even simpler, just add some helper CheckIfOk(wxD2DContext*) function that would do this and wrap all new wxD2DContext(...) calls in it.

A template New() is safer as it would make it impossible to use a context which couldn't be created.