Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Modify dxDraw(Material)Primitive to use same queue as other dxDraw functions #565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ccw808 merged 2 commits into multitheftauto:master from forkerer:SingleQueuePrimitives
Sep 27, 2018

Conversation

@forkerer
Copy link
Contributor

@forkerer forkerer commented Sep 22, 2018
edited by qaisjp
Loading

This pull requests is my suggestion of fixing #434.
Instead of using separate queues for primitives, they are now processed along with other dxDraws.
I had some problems with not being able to use smart pointers, due to the fact that draw queue items are stored as union, and they ain't that happy with unique/shared_ptr's. I would be glad if someone with more c++ experience looked over the code to confirm that it's the correct way of solving it.

There's a simple screenshot showing that the order is properly processed (rectangle > primitive >primitiveMaterial > rectangle):

image

This pull request also includes a bit of code cleanup for primitive batchers, as well as CLuaDrawingDefs.cpp primitive functions.

MIKI785 and AlexTMjugador reacted with thumbs up emoji

// Set states
m_pDevice->SetRenderState(D3DRS_ZENABLE, m_bZTest ? D3DZB_TRUE : D3DZB_FALSE);
m_pDevice->SetRenderState(D3DRS_ZENABLE, D3DZB_TRUE);
Copy link
Member

Choose a reason for hiding this comment

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

Depth-buffering is not required for 2D draws, so D3DRS_ZENABLE should be set to D3DZB_FALSE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about stuff like this: https://www.youtube.com/watch?v=CbJerQJBM0M where 2D images, or in this case primitives are placed in 3D world using shaders.

Copy link
Member

Choose a reason for hiding this comment

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

By convention, the 2D drawing functions draw in front of the final world render (The video shows the logo always in front of the car). So the depth buffer should be
disabled by default.

For drawing primitives in the 3D world, we could add dxDrawPrimitive3D (similar to dxDrawLine3D)

Copy link

@CrosRoad95 CrosRoad95 Sep 26, 2018

Choose a reason for hiding this comment

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

no we could, we must add 3d version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, disabled the depth buffering for material primitives :)

Copy link

ghost commented Sep 23, 2018

You did a very great job with refactoring code. Looks much better.

@patrikjuvonen patrikjuvonen changed the title (削除) Modify dxDraw(Material)Primitive to use same queue as other dxDraw functions. (削除ここまで) (追記) Modify dxDraw(Material)Primitive to use same queue as other dxDraw functions (追記ここまで) Sep 25, 2018
@ccw808 ccw808 merged commit 882f572 into multitheftauto:master Sep 27, 2018
Copy link
Member

ccw808 commented Sep 27, 2018

Thank you!

forkerer and StrixG reacted with thumbs up emoji

@qaisjp qaisjp added this to the 1.5.7 milestone Oct 2, 2018
@botder botder assigned forkerer and unassigned forkerer Oct 7, 2018
@patrikjuvonen patrikjuvonen added bug Something isn't working refactor and removed refactor labels Jan 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ccw808 ccw808 ccw808 left review comments

+1 more reviewer

@CrosRoad95 CrosRoad95 CrosRoad95 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working refactor

Projects

None yet

Milestone

1.5.7

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /