Skip to content

add swap effect COPY support in Swap(), enable BackBufferScale#2429

Open
jackchentwkh wants to merge 8 commits into
Cxbx-Reloaded:masterfrom
jackchentwkh:fix_backbufferscale
Open

add swap effect COPY support in Swap(), enable BackBufferScale#2429
jackchentwkh wants to merge 8 commits into
Cxbx-Reloaded:masterfrom
jackchentwkh:fix_backbufferscale

Conversation

@jackchentwkh
Copy link
Copy Markdown
Contributor

BackBufferScale sample works, at least with my LLE POC.

add BackBufferScale support
@github-actions github-actions Bot added HLE High Level Emulation graphics GPU and/or game graphics related labels Sep 4, 2023
@jackchentwkh
Copy link
Copy Markdown
Contributor Author

this PR is merely for reference in case anyone interested in how I did it.
the code is not polished and there are rendering jitters need to be fine tuned.

Copy link
Copy Markdown
Member

@RadWolfie RadWolfie left a comment

Choose a reason for hiding this comment

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

I understand the PR is merely for reference, however there's at least one issue with this reference PR. Such as CxbxrImpl_Swap get called from where?

Another issue is missing D3DDevice_GetBackBuffer_8 call. However, it can put on hold temporary until push into this pull request or future pull request.

At the moment, I had reviewed the code. The rest can be review by one of graphic experts.

Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
Comment thread src/core/hle/D3D8/Direct3D9/Direct3D9.cpp Outdated
/* pSourceRect = */ &scr,
/* pDestSurface = */ pXboxFrontBufferHostSurface,
/* pDestRect = */ nullptr,
/* Filter = */ D3DTEXF_LINEAR
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.

Perhaps make this filter style configurable, or add a comment explaining why LINEAR is chosen here (same applies below)

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.

frankly I have no idea about how to choose a proper filter here.

Copy link
Copy Markdown
Member

@PatrickvL PatrickvL Sep 6, 2023

Choose a reason for hiding this comment

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

Then perhaps add an end of line comment along the lines of // TODO: verify filtering mode with hardware behaviour

@jackchentwkh
Copy link
Copy Markdown
Contributor Author

update PR address almost all requests asked by reviewers except the configurable filter

@github-actions github-actions Bot added the modules submodules label Sep 5, 2023
@jackchentwkh
Copy link
Copy Markdown
Contributor Author

jackchentwkh commented Sep 5, 2023 via email

@jackchentwkh
Copy link
Copy Markdown
Contributor Author

the newest PR update enable master build and put the missing part of back buffer scale in effect inside GetMultiSampleScaleRaw() which I wrongly deleted when solving the merge conflict. it works in master without any jitter.

GetMultiSampleScaleRaw(scaleX, scaleY);
}

// the scales returned from GetMultiSampleScaleRaw(scaleX, scaleY) are already scaled with back buffer scales.
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.

Nit: Indentation mismatches surrounding lines


return result;
}
// ******************************************************************
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.

Nit: insert an empty line above

ret
}
}

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.

Nit: reduce 2 empty separator lines down to 1

CONST RECT* pDestRect,
PVOID pDummy1,
PVOID pDummy2
)
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.

Nit: unindent closing round brace. Same for below new function signatures.

LOG_FUNC_END;

//EMUPATCH(D3DDevice_Swap)(CXBX_SWAP_PRESENT_FORWARD); // Xbox present ignores
CxbxrImpl_Swap(CXBX_SWAP_PRESENT_FORWARD);
Copy link
Copy Markdown
Member

@PatrickvL PatrickvL Sep 6, 2023

Choose a reason for hiding this comment

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

Consider prefixing with /*ignore*/ (or whatever convention is being used elsewhere when a return value is consciously not being used)

IDirect3DSurface* pExistingHostRenderTarget = nullptr;
hRet = g_pD3DDevice->GetRenderTarget(0, &pExistingHostRenderTarget);
assert(hRet == D3D_OK);
// todo: there are jitters in stretching up, the source rect should be one possible cause.
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.

Nit: Always capitalize TODO: wording. The same for around 10+ lines below.

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

Labels

graphics GPU and/or game graphics related HLE High Level Emulation modules submodules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants