Skip to content

[Misc] Add GetRenderImage() and fix DrawPixmap() on renderimages#371

Draft
GWRon wants to merge 8 commits intobmx-ng:masterfrom
GWRon:feat_createRenderImage_enhancement
Draft

[Misc] Add GetRenderImage() and fix DrawPixmap() on renderimages#371
GWRon wants to merge 8 commits intobmx-ng:masterfrom
GWRon:feat_createRenderImage_enhancement

Conversation

@GWRon
Copy link
Copy Markdown
Contributor

@GWRon GWRon commented Mar 18, 2025

[Co-Authored by @davecamp aka Col]

@GWRon GWRon force-pushed the feat_createRenderImage_enhancement branch from a173264 to bcf6d7a Compare March 19, 2025 13:09
@GWRon
Copy link
Copy Markdown
Contributor Author

GWRon commented Mar 19, 2025

The PR also handles the following issues:

  • Alt-Tabbing in a fullscreen application utilizing DirectX 9 (would not update backbuffer or renderimages but throw messages around) and proper handling of device loss/reset
  • DrawPixmap outside of backbuffer regions and in render images in general (both also only issues for DirectX 9)

And it adds the ability to retrieve the currently set render image (if any)

@GWRon GWRon marked this pull request as draft March 19, 2025 16:28
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this in C++?

renderImagesIter appears to be global. How will this perform in a multithreaded environment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphic stuff needs to be called from the same thread (in bmax this will be the main thread...most likely).

So it should automatically be thread-safe ...if called properly. (Other max2d stuff is btw also relying on globals...).

You would prefer if it was made thread-safe?

Cpp: @davecamp used it as he said it was easier to write it in cpp ..but guess this would be transferable to C too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I think could also be done is: to have a "manager instance" which one could request ...and then passes around on each request done from within BlitzMax (so when adding, iterating removing). So it would be more visible (from POV of the BlitzMax-Developer/user) that it is their task to ensure thread-safety - IF required.

In other words: Mutexes would have to be placed inside of the max2d-drivers IF you were allowed to use the graphics context from multiple threads.

Is that something you (@woollybah) would prefer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried my best to kinda "convert" it to C .. it compiles but yeah, you are the C gurus.

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdbool.h>

#include <brl.mod/blitz.mod/blitz.h>


// Enable define if you want debug info printed
//#define OUTPUT_DEBUG_INFO


typedef struct Node {
    BBOBJECT* object;
    struct Node* next;
} Node;


static Node* renderImagesHead = NULL;
static Node* renderImagesIter = NULL;


void DebugOutput(const char* msg) {
    printf("%s\n", msg);
}


void DebugOutputObject(const char* msg, BBOBJECT* object) {
    printf("%s %p\n", msg, (void*)object);
}


#if defined(OUTPUT_DEBUG_INFO)
	#define DEBUG_OUTPUT(msg) DebugOutput(msg)
	#define DEBUG_OUTPUT_OBJECT(msg, obj) DebugOutputObject(msg, obj)
#else
	#define DEBUG_OUTPUT(msg) ((void)0)
	#define DEBUG_OUTPUT_OBJECT(msg, obj) ((void)0)
#endif


void RenderImageManager_AddRenderImage(BBOBJECT* object) {
    DEBUG_OUTPUT_OBJECT("RenderImageManager_AddRenderTarget - Adding render image:", object);
    
    Node* newNode = (Node*)malloc(sizeof(Node));
    if (!newNode) return;
    newNode->object = object;
    newNode->next = renderImagesHead;
    renderImagesHead = newNode;
    
    DEBUG_OUTPUT("RenderImageManager_AddRenderTarget - Render image list updated");
}


void RenderImageManager_RemoveRenderImage(BBOBJECT* object) {
    DEBUG_OUTPUT_OBJECT("RenderImageManager_RemoveRenderTarget - Removing render image:", object);
    
    Node** current = &renderImagesHead;
    while (*current) {
        if ((*current)->object == object) {
            Node* toDelete = *current;
            *current = (*current)->next;
            free(toDelete);
            DEBUG_OUTPUT("RenderImageManager_RemoveRenderTarget - Render image removed");

            return;
        }
        current = &((*current)->next);
    }
}


void RenderImageManager_ResetIterator() {
    DEBUG_OUTPUT("RenderImageManager_ResetIterator - Resetting iterator");
    renderImagesIter = renderImagesHead;
}


bool RenderImageManager_GetNextValue(BBOBJECT** pObject) {
    if (renderImagesIter) {
        *pObject = renderImagesIter->object;
        renderImagesIter = renderImagesIter->next;

        return true;
    }
    return false;
}

Copy link
Copy Markdown
Contributor Author

@GWRon GWRon Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be the "instance-ized" version (needs the creation and deletion from within BlitzMax to be added - and it is used from multiple objects, so at the end you had a global in there again ... or some more in depth code-restructuring which is ... which would be for DX9 ...):

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdbool.h>

#include <brl.mod/blitz.mod/blitz.h>


// Enable define if you want debug info printed
#define OUTPUT_DEBUG_INFO


typedef struct Node {
    BBOBJECT* object;
    struct Node* next;
} Node;


typedef struct RenderImageManager {
    Node* head;
    Node* iter;
} RenderImageManager;


void DebugOutput(const char* msg) {
    printf("%s\n", msg);
}


void DebugOutputObject(const char* msg, BBOBJECT* object) {
    printf("%s %p\n", msg, (void*)object);
}


#if defined(OUTPUT_DEBUG_INFO)
    #define DEBUG_OUTPUT(msg) DebugOutput(msg)
    #define DEBUG_OUTPUT_OBJECT(msg, obj) DebugOutputObject(msg, obj)
#else
    #define DEBUG_OUTPUT(msg) ((void)0)
    #define DEBUG_OUTPUT_OBJECT(msg, obj) ((void)0)
#endif


RenderImageManager* RenderImageManager_Create() {
    RenderImageManager* manager = (RenderImageManager*)malloc(sizeof(RenderImageManager));
    if (!manager)
        return NULL;

    manager->head = NULL;
    manager->iter = NULL;

    return manager;
}


void RenderImageManager_Destroy(RenderImageManager* manager) {
    if (!manager)
	return;

    Node* current = manager->head;
    while (current) {
        Node* toDelete = current;
        current = current->next;
        free(toDelete);
    }
    free(manager);
}


void RenderImageManager_AddRenderImage(RenderImageManager* manager, BBOBJECT* object) {
    if (!manager)
        return;

    DEBUG_OUTPUT_OBJECT("RenderImageManager_AddRenderTarget - Adding render image:", object);
    
    Node* newNode = (Node*)malloc(sizeof(Node));
    if (!newNode)
        return;
    
    newNode->object = object;
    newNode->next = manager->head;
    manager->head = newNode;
    
    DEBUG_OUTPUT("RenderImageManager_AddRenderTarget - Render image list updated");
}

void RenderImageManager_RemoveRenderImage(RenderImageManager* manager, BBOBJECT* object) {
    if (!manager)
        return;

    DEBUG_OUTPUT_OBJECT("RenderImageManager_RemoveRenderTarget - Removing render image:", object);
    
    Node** current = &(manager->head);
    while (*current) {
        if ((*current)->object == object) {
            Node* toDelete = *current;
            *current = (*current)->next;
            free(toDelete);
            DEBUG_OUTPUT("RenderImageManager_RemoveRenderTarget - Render image removed");

            return;
        }
        current = &((*current)->next);
    }
}

void RenderImageManager_ResetIterator(RenderImageManager* manager) {
    if (!manager) return;
    DEBUG_OUTPUT("RenderImageManager_ResetIterator - Resetting iterator");
    manager->iter = manager->head;
}

bool RenderImageManager_GetNextValue(RenderImageManager* manager, BBOBJECT** pObject) {
    if (!manager || !pObject)
        return false;

    if (manager->iter) {
        *pObject = manager->iter->object;
        manager->iter = manager->iter->next;

        return true;
    }
    return false;
}

Copy link
Copy Markdown
Contributor

@davecamp davecamp Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this in C++?

As in why not in bmax?

A list of TRenderImages is needed for persisting image data when transitioning between d3d9 device loss and device reset states (resolution changes, alt-tab etc). For device loss/reset to succeed all TRenderImage(s) need to be Release()'d before the d3d9->Reset method is called. After the reset succeeds all render images are recreated with their persisted data. This is sole the reason for the list.

Initially the list was in the d3d9graphics module in bmax code, which also meant that the gc has its eyes on the list and its contents. That does work ok except when a TRenderImage was no longer reachable from any bmax user code as the list has a reference to the render image. This meant that the user had to call a function to remove the render image from the list so that the Delete method can be called properly.

Moving the list to outside the eyes of the gc allows for any TRenderImages to call its destructor when no longer reachable/needed by user code. The Delete method can then remove itself from the list automatically without the need for the user to call a function to do so.

Copy link
Copy Markdown
Contributor

@davecamp davecamp Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multithreading...
There are locks in the bmax code due to the gc being multithreaded (optionally it seems), where another thread calls finalisers and therefore the Delete method from outside the main thread.

This now, quite rightly, raises the question of calling Release on d3d9 resources from other threads. If its a problem normally it would cause an EAV or undefine behaviour, and the docs do say not to do this without the D3DCREATE_MULTITHREADED flag being used in the d3d9->CreateDevice method.
This scenario, of a thread releasing d3d9 resources that is not the main thread, was already happening before these changes. Maybe it's something else that should be battened down too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/ivmai/bdwgc/blob/master/docs/finalization.md

Finalization code may be run anyplace an allocation or other call to the collector takes place. In multi-threaded programs, finalizers have to obey the normal locking conventions to ensure safety. Code run directly from finalizers should not acquire locks that may be held during allocation. This restriction can be easily circumvented by calling GC_set_finalize_on_demand(1) at program start and creating a separate thread dedicated to periodic invocation of GC_invoke_finalizers().

In single-threaded code, it is also often easiest to have finalizers queued and, then to have them explicitly executed by GC_invoke_finalizers().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants