replace coloring code with coloring plugin by lucky01 · Pull Request #347 · freezy/dmd-extensions

lucky01 · 2022-11-24T15:51:13Z

Hi Freezy,

as recommended from you here is the pull request with the code changes necessary to make the pin2dmd coloring plugin with pac support work. Also some fixes for pindmdV3 and alphanumeric devices integrated.

(cherry picked from commit 47a3297)

(cherry picked from commit 279e811)

(cherry picked from commit 48a3250)

…Gray2, Gray4 sources

freezy · 2023-05-13T20:07:28Z

Only if frame.width and frame.height matches the size of the frame data, which is not the case if HDScaling is active.

Yeah, I debugged into it, and indeed if HD scale is set, the frame size doesn't correspond to the data. I would consider this as a bug, and the scaling should be done after the colorization, if the colorizer doesn't already upscale it. Something like this:

Would that make sense?

lucky01 · 2023-05-14T08:06:31Z

Either there are a lot of paths missing or wrong in your graph or I just don´t get it.
Anyway, scaling IS done after colorization already and the width/height parameters are just taken to indicate that scaling is needed. I´m ok how it is but if you consider this as a bug, feel free to change it.

freezy · 2023-05-29T10:23:31Z

+		public enum ColorizerMode
+		{
+			None = -1,
+			SimplePalette = 0,
+			Advanced128x32 = 1,
+			Advanced192x64 = 3,
+			Advanced256x64 = 4,
+		}


I'm testing SAM games (ACDC), and I can't get it to work. I'm posting this here and not on my PR, since it doesn't seem to work here, either. I think the problem is that there is no way of retrieving the palette changes from the plugin.

About the code above: Could you confirm (or correct me) that:

None means that no colorization files were found, thus nothing will be colored. There will be a way to forward frames to the plugin nevertheless (for frame dumping), but nothing is expected in return, i.e. the frame will be routed from the original source to the destinations, and not from the converter.

SimplePalette means that we'll only get palette changes from the plugin, for example for ACDC with a patched ROM. Here, we would retrieve the new palette from the plugin and re-emit the original frame with that palette.

Advanced128x32 means that all frames are colored and returned by the plugin, at 128x32.

Advanced192x64 also means that all frames come from the plugin, at 192x64.

Advanced256x64 same as above, but at 256x64.

lucky01 · 2023-05-29T12:30:36Z

Simple Palette means a pal is found but no pac/vni/fsq for advanced coloring. It also returns the colorized frame in the same resolution as the source.

freezy · 2023-05-29T12:34:55Z

Right, so ACDC with only a .pal should return SimplePalette, right? (It returns None.)

lucky01 · 2023-05-29T12:43:14Z

It is not a simple palette but multiple palettes with ROM based palette switching. I don't remember what it reports back but I think it should be simple palette.

lucky01 · 2023-05-29T12:44:48Z

You need to make sure the console data is forwarded to the plugin

freezy · 2023-05-29T21:30:15Z

It is not a simple palette but multiple palettes with ROM based palette switching.

Yes, so how about a MultiPalette mode, where dmdext can query the plugin for the palette with GetCurrentPalette() (returning an array of colors)?

You need to make sure the console data is forwarded to the plugin

Yes, that's the case.

lucky01 · 2023-05-30T07:11:44Z

Yes, so how about a MultiPalette mode, where dmdext can query the plugin for the palette with GetCurrentPalette() (returning an array of colors)?

I think that is not worth the time since there are only a hand full of smartdmd tables which will be converted anyway (e.g. Metallica)

freezy · 2023-05-30T07:14:24Z

I think that is not worth the time since there are only a hand full of smartdmd tables which will be converted anyway (e.g. Metallica)

I don't understand.. There are already colored games, how do you want to support them otherwise?

lucky01 · 2023-05-30T08:46:45Z

I don't understand.. There are already colored games, how do you want to support them otherwise?

I updated your testing DLL in dropbox. It was reporting the wrong colorization type.

freezy · 2023-05-30T08:48:58Z

Cool, now how do I get the new palettes?

lucky01 · 2023-05-30T09:27:22Z

You get the colored RGB24Am 30.05.2023 um 11:49 schrieb freezy ***@***.***>: Cool, now how do I get the new palettes? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

freezy · 2023-05-30T09:46:37Z

That sucks. What you're proposing is:

Your DLL converts a gray4-frame to RGB24 based on the current palette (expensive)
dmdext extracts the palette from the RGB24 frame (expensive)
dmdext creates a new colored frame with original frame data and the extracted palette (cheap)

Instead of:

Your DLL just returns the palette (cheap)
dmdext adds the palette to the frame (cheap)

lucky01 · 2023-05-30T10:00:17Z

That is what we agreed on . My plugin returns colorized RGB24 frames. Am 30.05.2023 um 12:46 schrieb freezy ***@***.***>: That sucks. What you're proposing is: Your DLL converts a gray4-frame to RGB24 based on the current palette dmdext extracts the palette from the RGB24 frame dmdext creates a new colored frame with original frame data and the extracted palette Instead of: Your DLL just returns the palette dmdext adds the palette to the frame —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

freezy · 2023-05-30T11:46:25Z

Well, technically we agreed on not sending colored grayX-frames for pin2color-generated frames. Here, the palette changes are triggered by the ROM.

I'm trying to understand your motivation for such a shitty solution, because I can't see any benefits for anyone.

lucky01 · 2023-05-30T14:50:31Z

Technically those ARE colored gray X (4) frames generated by the pin2color plugin and therefore returned as RGB24.All colored frames returned by the plugin are RGB24 . That is the consent we have and that is how it is implemented in my pull request.Am 30.05.2023 um 14:46 schrieb freezy ***@***.***>: Well, technically we agreed on not sending colored grayX-frames for pin2color-generated frames. Here, the palette changes are triggered by the ROM. I'm trying to understand your motivation for such a shitty solution, because I can't see any benefits for anyone. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

freezy · 2023-05-30T15:23:41Z

Apart from whatever you think the "consent" is, would you agree that it's a bad solution with no benefits for anybody involved? And if so, would you help improving the solution? And it not, could you elaborate the benefits you see?

lucky01 · 2023-05-30T16:12:26Z

Intensive tests of my plugin solution have been made by the community and no drawback of any kind has been reported.
It seems all users of my plugin solution are happy.
The only thing I can agree with you is that we disagree on every point we discuss, Makes no sense to start the discussion again.

freezy · 2023-05-30T17:26:36Z

Intensive tests of my plugin solution have been made by the community

But you've just changed the entire API. Nobody has ever tested what we're discussing here. This argument is nonsense.

The only thing I can agree with you is that we disagree on every point we discuss, Makes no sense to start the discussion again.

Collaborator of the year ;)

Look, I'm not particularly enjoying this either. But I'm trying to put my feelings away so we can find a good solution. A good solution would be to retrieve the palette, so we can simply apply it to the frame and be done with it, instead of looping through the byte array multiple times.

lucky01 · 2023-05-30T20:24:00Z

But you've just changed the entire API. Nobody has ever tested what we're discussing here. This argument is nonsense.

Since may the 3rd the version which is for download on VPU and intensively tested is based on the latest code on my fork and IS using exactly the same API like you.

freezy · 2023-05-31T20:38:38Z

Since may the 3rd the version which is for download on VPU and intensively tested is based on the latest code on my fork and IS using exactly the same API like you.

That's what happens when using this PR with the pin2color.dll from VPU:

 [1] 2023/05/31 22:27:39.110  INFO | [Pin2Color] Successfully loaded colorizer plugin ... 
 [1] 2023/05/31 22:27:39.155  INFO | [Pin2Color] Failed to open colorizer ...

Which makes sense, since yourself said:

I sent you a dll version for testing. I refactored the coloring plugin interface to use the BSD-3 licensed dmddevice.dll api.

So, it's a different API, which is fine, it's just not the one the community tested.

Anyway, I'll be soon releasing a beta version to test, so more people can check for regressions. The pin2color.dll testing will be done when you're back from holidays, I guess.

About alphanumeric colorizations: Is there anything I could test with? Do you have one yourself that you could test?

freezy · 2023-06-18T16:16:04Z

+		private static void processEvent()
+		{
+			if (_activePinUpOutput != null) {
+				uint eventID = ColorizeGetEvent();
+				if (eventID != lastEventID) {
+					lastEventID = eventID;
+					_activePinUpOutput.SendTriggerID((ushort)eventID);
+				}
+			}
+		}


AFAIK this is for an unreleased PinUp feature that was added to Serum recently. Did you just copy it blindly, or is there a way I can test this?

Joerg Amann added 30 commits May 15, 2022 20:54

fix crash with pindmdV3

2895034

add scalers to RGB24 output

a444a99

fix RGB output on pin2dmd when scaling is enabled

4c4604b

activate pixelCade

6bc07ce

(cherry picked from commit 47a3297)

remove outdated coloring code

ba8fc11

(cherry picked from commit 279e811)

remove more outdated code

e3ffb44

remove Gray6Destination from pin2dmd

9a07681

remove gray6destinations

a1009e1

remove unneeded variables

aa1e59c

(cherry picked from commit 48a3250)

fix TestCommand

7d4f955

add DLL calls

3db450d

add console data to colorize API

a855551

add colorizer Interface for raw frames

6587dc8

add additional calls

b512d8e

add additional calls

cce558a

first working implemetation of plugin DLL

94efa62

enumerate colorizer mode

6689364

add colored RGB24 as IDestination and Frameformat

d415d3c

add IDestination to project

48bbfc9

forward colored frame to ColoredRgb24Destination

bde9861

activate pinup renderer

5cbdce0

add coloredRgb24Destination to pindmd3 / pin2dmd / Pixelcade

1bbc109

skip only gray2 and gray4 if coloring is available

0143a24

forward palette and console data to colorizer

33daaef

set palette from pinmame colors

25a198d

remove unneeded IColoredGrayXDestinations

3d60bd3

fix rawframe triggers

70487fc

rename IColoredRgb24Destination to IColoredGrayDestination

02ffb00

rename IColoredRgb24Source to IColoredGraySource and remove unneeded …

b07cbb9

…Gray2, Gray4 sources

catch nullpointer exception

6d95383

lucky01 added 4 commits May 23, 2023 10:42

Merge remote-tracking branch 'upstream/master' into coloring_plugin

0079c9c

disable event keyframe forwarding to pup for now

c6ceb0e

remove unneeded variable

0b87987

fix typo

a7961e4

freezy mentioned this pull request May 27, 2023

Add plugin support and refactor render graph. #374

Merged

freezy reviewed May 29, 2023

View reviewed changes

freezy reviewed Jun 18, 2023

View reviewed changes

fix method signatures

ef4a3fd

freezy closed this in #374 Aug 13, 2023

Conversation

lucky01 commented Nov 24, 2022

Uh oh!

freezy commented May 13, 2023

Uh oh!

lucky01 commented May 14, 2023

Uh oh!

freezy May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucky01 commented May 29, 2023 via email • edited by freezy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

freezy commented May 29, 2023

Uh oh!

lucky01 commented May 29, 2023 via email • edited by freezy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucky01 commented May 29, 2023 via email • edited by freezy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

freezy commented May 29, 2023

Uh oh!

lucky01 commented May 30, 2023

Uh oh!

freezy commented May 30, 2023

Uh oh!

lucky01 commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

freezy commented May 30, 2023

Uh oh!

lucky01 commented May 30, 2023 via email

Uh oh!

freezy commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucky01 commented May 30, 2023 via email

Uh oh!

freezy commented May 30, 2023

Uh oh!

lucky01 commented May 30, 2023 via email

Uh oh!

freezy commented May 30, 2023

Uh oh!

lucky01 commented May 30, 2023

Uh oh!

freezy commented May 30, 2023

Uh oh!

lucky01 commented May 30, 2023

Uh oh!

freezy commented May 31, 2023

Uh oh!

freezy Jun 18, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

freezy May 29, 2023 •

edited

Loading

lucky01 commented May 29, 2023 via email •

edited by freezy

Loading

lucky01 commented May 29, 2023 via email •

edited by freezy

Loading

lucky01 commented May 29, 2023 via email •

edited by freezy

Loading

lucky01 commented May 30, 2023 •

edited

Loading

freezy commented May 30, 2023 •

edited

Loading