Skip to content

Support TinyVG format decoding and rendering #77

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
merged 1 commit into from
Dec 19, 2024

Conversation

ndsl7109256
Copy link
Collaborator

@ndsl7109256 ndsl7109256 commented Dec 5, 2024

Implemented support for decoding TinyVG format by parsing commands step-by-step. Each decoded command is executed using twin's path functions to render graphics. This enhances twin's capability to handle compact vector graphics .

2024-12-09.3.20.24.mov

Validating with TinyVG examples (Left : Origin, Right : Rendered by Mado)

Ref:
https://tinyvg.tech/download/specification.pdf

Close #71

@ndsl7109256 ndsl7109256 force-pushed the tvg branch 2 times, most recently from 0c18345 to cd9554e Compare December 5, 2024 03:54
@jserv jserv requested a review from weihsinyeh December 5, 2024 04:48
@jserv jserv requested review from Bennctu and alanjian85 December 5, 2024 05:01
@ndsl7109256 ndsl7109256 force-pushed the tvg branch 5 times, most recently from 330f7a8 to 88edea8 Compare December 5, 2024 06:08
@jserv
Copy link
Contributor

jserv commented Dec 5, 2024

Summarize the rendering of TinyVG by validating the examples provided by TinyVG website.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

The tiger SVG is a standard demonstration image for SVG viewers, which can be set as the default viewer image in TinyVG rendering.

@jserv
Copy link
Contributor

jserv commented Dec 5, 2024

The font rasterization in TinyVG image does not precisely match the rendered flow chart. Could you verify?

src/path.c Outdated
@@ -369,6 +369,7 @@ void twin_path_arc_ellipse(twin_path_t *path,
param.extent);

twin_path_set_matrix(path, save);
twin_path_draw(path, target_x, target_y);
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't get it. Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

I think there might be some error in the arc ellipse calculation, so I enforced drawing to the target point to ensure correctness.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there might be some error in the arc ellipse calculation, so I enforced drawing to the target point to ensure correctness.

I would expect that we can overcome the underlying issues before the merging of this pull request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

I think the accuracy of twin_acos could be improved. When I use the acos function from the standard math library, the results seem better. To enhance the accuracy of the arctan calculation, I try to use more precise angle values (0-32768) which allows us perform additional iterations for the CORDIC algorithm.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jouae, Please comment the above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ndsl7109256 could you provide how you use math.h to get the conclusion, along with a comparison between math.h and current twin_acos ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just replace twin_acos with acos in vector_angle and check the rendering result.Besides, I evaluated the CORDIC algorithm's error for different iteration count(original and modified) by testing all integer pairs $(x,y)$, $x \in [1,15]$, $y \in [1,15]$ For each pair, I calculated the arctangent using both the CORDIC method (with varying iterations) and the atan2 from math library.

Iteration 12 15
avg error 0.071239 0.011101
std dev 0.054116 0.008201
max error 0.284898 0.043332

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ndsl7109256

I found you can compute the angle between two vectors without any sqrt operations.

Just modify vector_angle function as:

    twin_fixed_t dot = twin_fixed_mul(ux, vx) + twin_fixed_mul(uy, vy);
+   twin_fixed_t det = twin_fixed_mul(ux, vy) - twin_fixed_mul(uy, vx);
+   twin_fixed_t angle = twin_atan2(det, dot);

You can derive how to calculate the angle based on the following relationship

$$ \tan{\theta} = \frac{\sin{\theta}}{\cos{\theta}} = \frac{\text{det}(u,v)}{u \cdot v} $$

$$ \cos{\theta} = \frac{u \cdot v}{|u||v|} $$

$$ \sin{\theta} = \frac{\text{det}(u,v)}{|u||v|} $$

where $\text{det}$ is the determinant of $u$ and $v$, for more detail see Wikpedia: Determinant.

The advantage of this formula is that it avoids calculating the lengths of $u$ and $v$, thereby eliminating the need for the sqrt operation.

However, this might not be a solution to the rendering problem. The issue is likely caused by insufficient precision, leading to rendering errors. Further detailed analysis is required to obtain a more accurate conclusion and potential solutions.

Copy link
Collaborator Author

@ndsl7109256 ndsl7109256 Dec 11, 2024

Choose a reason for hiding this comment

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

Your formula looks great, but the result looks bad :(
image
Could you help me verify them?

@ndsl7109256
Copy link
Collaborator Author

ndsl7109256 commented Dec 6, 2024

The font rasterization in TinyVG image does not precisely match the rendered flow chart. Could you verify?

image

I found that when I zoom in on the flow chart, the font rasterization issue disappears. Maybe we can check the filling function later.

uint8_t g;
uint8_t b;
uint8_t a;
} tvg_rgba32_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

typedef uint32_t twin_argb32_t;
Can we reuse the previous implementation? I'm uncertain if it's better to stick with the previous type by converting from float to uint8_t when the pixel format is float.

src/image-tvg.c Outdated
return res;
}
res = tvg_read_unit(ctx, &out_header->line_width);
if (res != TVG_SUCCESS) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Within the function, it can check whether res is TVG_SUCCESS. If res is not TVG_SUCCESS, the function can return res immediately. Otherwise, at the end of the function, it can return res directly without checking whether res is TVG_SUCCESS.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Create a user-friendly file selection interface for TinyVG vector graphics using buttons, enabling seamless file picking. Conduct thorough validation of the TinyVG image processing capabilities to ensure the usability.

Then, we can explore potential contributions to established embedded graphics libraries like LVGL, which currently uses ThorVG for vector rendering. The goal is to develop a more efficient graphics solution that excels in resource efficiency and can operate effectively in environments without FPU support.

@ndsl7109256 ndsl7109256 force-pushed the tvg branch 2 times, most recently from 0137be5 to 8570e00 Compare December 9, 2024 07:08
@ndsl7109256 ndsl7109256 force-pushed the tvg branch 5 times, most recently from cb60903 to 663c061 Compare December 10, 2024 03:06
@jserv jserv requested a review from weihsinyeh December 12, 2024 13:44
out_style->flat = flat;
break;
case TVG_STYLE_LINEAR:
res = tvg_parse_gradient(ctx, &grad);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does line 532 require checking if res equals TVG_SUCCESS, while lines 538 and 542 do not?
Should a log_error be added if they require it?

int kind,
tvg_style_t *out_style)
{
tvg_result_t res;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I cannot understand why res will be checked initially in line 557, but res will not be checked here?

    uint32_t u32;
    tvg_result_t res = tvg_read_varuint(ctx, &u32);
    if (res != TVG_SUCCESS) {
        return res;
    }

if (res != TVG_SUCCESS) {
goto error;
}
error:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the following code snippet to address memory leaks related to ctx.path.

error:
    if (ctx.path) {
    	twin_path_destroy(ctx.path);
    	ctx.path = NULL;
    }
    if (ctx.colors != NULL) {
        free(ctx.colors);
        ctx.colors = NULL;
        ctx.colors_size = 0;
    }

@ndsl7109256 ndsl7109256 force-pushed the tvg branch 2 times, most recently from 2662bd1 to dee3886 Compare December 17, 2024 08:17
@jserv jserv requested a review from weihsinyeh December 17, 2024 17:11
tvg_result_t res;

/* Current implementation only produces TWIN_ARGB32 */
if (fmt != TWIN_ARGB32) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should log_error be added here?

@ndsl7109256 ndsl7109256 force-pushed the tvg branch 2 times, most recently from a752707 to 2a4ac89 Compare December 18, 2024 06:04
Implemented support for decoding TinyVG format by parsing commands
step-by-step. Each decoded command is executed using twin's path
functions to render graphics. This enhances twin's capability to handle
compact vector graphics .

Ref:
https://tinyvg.tech/download/specification.pdf

Close sysprog21#71
@jserv jserv merged commit 3bee378 into sysprog21:main Dec 19, 2024
3 checks passed
@jserv
Copy link
Contributor

jserv commented Dec 19, 2024

Thank @ndsl7109256 for contributing!

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.

Add TinyVG Support
5 participants