Skip to content
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

Add SVG support (lunasvg) #2026

Merged
merged 113 commits into from Sep 26, 2021
Merged

Add SVG support (lunasvg) #2026

merged 113 commits into from Sep 26, 2021

Conversation

Lpsd
Copy link
Member

@Lpsd Lpsd commented Jan 18, 2021

I've implemented lunasvg, which gives us the ability to create SVG documents, edit them on the fly, and also load SVG files (from path or raw).

API (for initial release)

svg svgCreate(int width, int height [, string pathOrRawData, function callback ])
xmlnode svgGetDocumentXML(svg svgElement)
bool svgSetDocumentXML(svg svgElement, xmlnode svgDocument [, function callback ])
int, int svgGetSize(svg svgElement)
bool svgSetSize(svg svgElement, int width, int height [, function callback ])

callback is for when SVG has been updated, invoked when texture is available (if update successful).

Example usage

local square = svgCreate(300, 300)
local squareXML = svgGetDocumentXML(square)

-- Add a rect svg node
local rect = xmlCreateChild(squareXML, "rect")
xmlNodeSetAttribute(rect, "x", "0")
xmlNodeSetAttribute(rect, "y", "0")
xmlNodeSetAttribute(rect, "width", "100%")
xmlNodeSetAttribute(rect, "height", "100%")
xmlNodeSetAttribute(rect, "fill", "red")

-- Apply our changes
svgSetDocumentXML(square, squareXML)

-- Create a square with blank document
local clone = svgCreate(300, 300)

-- Set the fill to blue
xmlNodeSetAttribute(rect, "fill", "blue")

-- Apply xml to blank svg document
svgSetDocumentXML(clone, squareXML)

addEventHandler("onClientRender", root, function()
    dxDrawImage(500, 150, 300, 300, square, 0, 0, 0, tocolor(255, 255, 255), false)
    dxDrawImage(900, 350, 300, 300, clone, 0, 0, 0, tocolor(255, 255, 255), false)
end)
local monkey = svgCreate(500, 500, "monkey.svg")

addEventHandler("onClientRender", root, function()
    dxDrawImage(0, 0, 500, 500, monkey, 0, 0, 0, tocolor(255, 255, 255), false)
end)
Preview of the first example

image

Preview of the second example

image

The svg element is also derived from CClientTexture, meaning it works with pretty much all dx* functions, including dxGetTexturePixels, dxSetTexturePixels, dxSetPixelColor, etc - direct manipulation is therefore possible, e.g:

local svg = svgCreate(300, 300, "svg.svg")

-- Bind to clear the SVG texture
bindKey("r", "down", function()
    local pixels = dxGetTexturePixels(svg)
    local width, height = svgGetSize(svg)
    
    for i = 0, width do
        for j = 0, height do
            dxSetPixelColor(pixels, i, j, 0, 0, 0, 0)
        end
    end

    dxSetTexturePixels(svg, pixels)
end)

(Resolves #1300)

@prnxdev
Copy link

prnxdev commented Jan 19, 2021

Nice!!

I think there is no need for new function svgCreate for files. I think that DxCreateTexture should work with .svg too the same way it works for .png or .jpg. Also OOP support would be nice :)

@Lpsd
Copy link
Member Author

Lpsd commented Jan 19, 2021

Integrating into the DX related code would not be good for the overall structure of the API.

svgCreate returns a texture element, CClientVectorGraphic, which can be passed directly to dxDrawImage.

CClientVectorGraphic encapsulates all of the SVG related data, the D3D9 render item and the graphics display class. None of the other dx* functions follow this convention, and in reality they're two completely separate constructs. It would be a mess to implement both features side by side.

Not to mention, we're going to have a LOT of functions with the svg* prefix, to manipulate the SVG document and load data. Somehow, dxCreateSVGDocument doesn't seem right to me?

Have a look into the code, to get a better understand of the implementation.

@Lpsd
Copy link
Member Author

Lpsd commented Jan 19, 2021

As briefly discussed on Discord, I think the initial plan should be to roll this out with a few basic functions:

texture/svg svgCreate(width, height [, string pathOrRawData ])
xmlnode svgGetDocumentXML(svg svgElement)
bool svgSetDocumentXML(xmlnode xmlData)

You can create a blank SVG document, supply a file path or read from raw (including xmlnode) data. You can also get the SVG document back as an xmlnode, use the existing xml implementation to edit the xml data, and set it back on the document.

The reason for introducing this simple behaviour at first, is because a proper API will take a lot of time and thought, and will probably be quite an addition to the code. Introducing the basics in this PR, and then the API stuff later, is easier on the reviewer(s).

We can always deprecate svgGet/SetDocumentXML at a later date, once a better API has been introduced.

Thoughts?

@patrikjuvonen patrikjuvonen added enhancement New feature or request upstream Related to vendor library labels Jan 19, 2021
@patrikjuvonen patrikjuvonen added this to In progress in Vendor upgrades via automation Jan 19, 2021
Client/core/Graphics/CRenderItemManager.cpp Outdated Show resolved Hide resolved
Client/core/Graphics/CRenderItemManager.cpp Outdated Show resolved Hide resolved
Client/mods/deathmatch/StdInc.h Outdated Show resolved Hide resolved
Client/mods/deathmatch/StdInc.h Outdated Show resolved Hide resolved
Client/mods/deathmatch/logic/CClientVectorGraphicDisplay.h Outdated Show resolved Hide resolved
Copy link
Contributor

@patrikjuvonen patrikjuvonen left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! I have now reviewed the code and overall looks good!

I saw a lot of code style formatting issues, are you running clang-format on your editor? We also have a clang-formatter batch script in the utils folder, which helps with fixing formatting issues to be consistent with the rest of the codebase.

Either way to speed up the process I decided to run clang-format on this PR and push the changes here.

Only one thing bugs me which is that the callback and non-callback code is duplicated, which is not best practice. I would like to see these refactored into a single private function call for example, or something else that doesn't result in duplicating code, thus duplicating work and duplicating amount of potential issues.

The size functions work great, thank you for adding them!

I think once you've refactored the duplicate code, this is ready for a final review and then merge.

@Lpsd
Copy link
Member Author

Lpsd commented Sep 25, 2021

Thanks for the review - I've split that code into static, private methods as suggested. In some cases it doesn't cut down the amount of code needed but at least it's more consistent for any future changes.

@Lpsd
Copy link
Member Author

Lpsd commented Sep 25, 2021

I've had to format the CLuaShared::GetAsyncTaskScheduler()->PushTask calls by hand, clang-format makes them really ugly if the task function only contains a single line (like a return). Rest should be fine

@patrikjuvonen patrikjuvonen removed the request for review from sbx320 September 26, 2021 11:29
Copy link
Contributor

@patrikjuvonen patrikjuvonen left a comment

Choose a reason for hiding this comment

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

Thank you very much, everything looks good now! 😄

@patrikjuvonen patrikjuvonen merged commit 201de35 into multitheftauto:master Sep 26, 2021
Vendor upgrades automation moved this from In progress to Done Sep 26, 2021
@patrikjuvonen patrikjuvonen removed this from Done in Vendor upgrades Sep 26, 2021
Dutchman101 pushed a commit that referenced this pull request Sep 26, 2021
patrikjuvonen added a commit that referenced this pull request Sep 26, 2021
This reverts commit 8d800eb.
This reverts commit aca6c1f.
This reverts commit 201de35.
patrikjuvonen added a commit that referenced this pull request Sep 26, 2021
patrikjuvonen added a commit that referenced this pull request Sep 27, 2021
patrikjuvonen added a commit that referenced this pull request Sep 27, 2021
patrikjuvonen added a commit that referenced this pull request Sep 27, 2021
* Revert "Revert "Add SVG support (lunasvg) (#2026)""

This reverts commit 898fcaf.

* Change SharedLib to StaticLib in premake5.lua

Co-authored-by: LopSided <40902730+Lpsd@users.noreply.github.com>
TheNormalnij pushed a commit to TheNormalnij/mtasa-blue that referenced this pull request Oct 20, 2021
TheNormalnij pushed a commit to TheNormalnij/mtasa-blue that referenced this pull request Oct 20, 2021
TheNormalnij pushed a commit to TheNormalnij/mtasa-blue that referenced this pull request Oct 20, 2021
This reverts commit 8d800eb.
This reverts commit aca6c1f.
This reverts commit 201de35.
TheNormalnij pushed a commit to TheNormalnij/mtasa-blue that referenced this pull request Oct 20, 2021
TheNormalnij pushed a commit to TheNormalnij/mtasa-blue that referenced this pull request Oct 20, 2021
TheNormalnij pushed a commit to TheNormalnij/mtasa-blue that referenced this pull request Oct 20, 2021
…ltitheftauto#2380)

* Revert "Revert "Add SVG support (lunasvg) (multitheftauto#2026)""

This reverts commit 898fcaf.

* Change SharedLib to StaticLib in premake5.lua

Co-authored-by: LopSided <40902730+Lpsd@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request upstream Related to vendor library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for SVG
7 participants