Skip to content

Conversation

@soburi
Copy link
Member

@soburi soburi commented Jan 13, 2023

This PR intends to use cfb like as a graphics oriented devices. It will be fundamental for implements simple graphics rendering.

API improvement:

  • Allow printing and inverting at any coordinates.

    Currently, the CFB API can only draw at tile bounds coordinates (where the y coordinate is a multiple of 8).
    Implements dot-by-dot font rendering to make cfb_print() able to draw text even at a coordinate that is not on tile boundaries.
    For API consistency, change cfb_invert_area() able to invert even at a coordinate not on tile boundaries.

  • Add cfb_draw_text() API

    Added cfb_draw_text() that does not wrap when specifying the position in dots, as it may be inconvenient if there is wrapping

Shell Improvements:

  • Add command draw text

    Add command to run the new API cfb_draw_text().
    This command is similar to print.
    The position specify with (x,y) not column-row and doesn't clear buffer is differences.

  • Add support for running cfb_invert_area().

    Extend the invert command to run cfb_invert_area when given four arguments.
    Make a change to invert the screen immediately in case of run invert without arguments.

@soburi soburi requested a review from jfischer-no as a code owner January 13, 2023 23:15
@soburi soburi force-pushed the cfb_render_on_any_coord branch 2 times, most recently from af1b4db to 786bcc2 Compare January 14, 2023 15:12
@soburi soburi force-pushed the cfb_render_on_any_coord branch 4 times, most recently from 45dbf1a to f925bbe Compare January 14, 2023 23:47
@MaureenHelm
Copy link
Member

@jfischer-no will you please have a look?

subsys/fb/cfb.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

static uint8_t draw_char_vtmono(const struct char_framebuffer *fb,
				char c, uint16_t x, uint16_t y,
				bool draw_bg)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for reviewing.

Align the line start.

subsys/fb/cfb.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the benefit of using WRITE_BIT() here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It make a sense.

I rewrite this code to run less times loop.

subsys/fb/cfb.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have short and concise comments on few points, otherwise no one can understand what happens at first glance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Add explaining the purpose of each in the process branching.

@soburi soburi force-pushed the cfb_render_on_any_coord branch 5 times, most recently from 9fd05cf to d0e71f3 Compare February 19, 2023 07:14
@soburi soburi requested a review from jfischer-no February 20, 2023 09:35
@soburi soburi force-pushed the cfb_render_on_any_coord branch from d0e71f3 to 63f60ab Compare February 24, 2023 13:57
jfischer-no
jfischer-no previously approved these changes Feb 24, 2023
Copy link

Choose a reason for hiding this comment

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

Would be nice with const char *str!? cfb_print should probably also have const parameter IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for reviewing.
It makes sense.
The function doesn't modify the pointer address and contents.
So, I change the type to const char *const.

soburi added 7 commits March 2, 2023 22:57
cfb_framebuffer_finalize() invert the framebuffer, and it still
leave as inverted after executing the function.

It restores the framebuffer at the end of the cfb_framebuffer_finalize()
for the continued drawing.

Signed-off-by: TOKITA Hiroshi <[email protected]>
cfb_print does not modify either address and contents of
the pointer that is pointing drawing text.
Thus, it can add a const modifier.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Implements dot-by-dot font rendering to make cfb_print() able to
draw text even at a coordinate that is not on tile boundaries.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Add cfb_draw_text() API for rendering text.
It is similar to cfb_print(), the difference is cfb_draw_text() accepts
coordinate that is not on tile boundaries and don't wrap the line.

Signed-off-by: TOKITA Hiroshi <[email protected]>
…e tile

Improve `cfb_invert_area()` able to invert even at a coordinate
not on tile boundaries.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Add the `draw text` command to execute the `cfb_draw_text()` API.

The command is similar to the `print` command. The difference is
`draw text` can render to coordinate that is not on tile boundaries.
The command is not run `cfb_framebuffer_clear()`,
So it allows rendering appendiceal with keeping already drawn image.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Extend the invert command to run cfb_invert_area when given four arguments.
Make a change to invert the screen immediately in case of run invert
without arguments.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi soburi force-pushed the cfb_render_on_any_coord branch from 63f60ab to 75eda27 Compare March 2, 2023 14:28
@soburi soburi requested a review from 5frank March 2, 2023 14:32
@teburd
Copy link
Contributor

teburd commented Mar 2, 2023

@jfischer-no @5frank please look again

@soburi
Copy link
Member Author

soburi commented Mar 8, 2023

@5frank
ping

@MaureenHelm
Copy link
Member

@fabiobaltieri please take a look

@carlescufi carlescufi merged commit 47f14d6 into zephyrproject-rtos:main Apr 6, 2023
@soburi soburi deleted the cfb_render_on_any_coord branch May 28, 2023 11:51
soburi added a commit to soburi/zephyr that referenced this pull request Jan 21, 2024
I broke the rendering of the CFB_FONT_MSB_FIRST font in
the previously committed zephyrproject-rtos#53796.
I will correct the entire font rendering process to make it
easier to remove the restriction that the vertical size of
the font must be a multiple of 8.

Signed-off-by: TOKITA Hiroshi <[email protected]>
soburi added a commit to soburi/zephyr that referenced this pull request Mar 3, 2024
I broke the rendering of the CFB_FONT_MSB_FIRST font in
the previously committed zephyrproject-rtos#53796.
I will correct the entire font rendering process to make it
easier to remove the restriction that the vertical size of
the font must be a multiple of 8.

Signed-off-by: TOKITA Hiroshi <[email protected]>
nashif pushed a commit that referenced this pull request Jun 28, 2024
I broke the rendering of the CFB_FONT_MSB_FIRST font in
the previously committed #53796.
I will correct the entire font rendering process to make it
easier to remove the restriction that the vertical size of
the font must be a multiple of 8.

Signed-off-by: TOKITA Hiroshi <[email protected]>
DashingR pushed a commit to tsisw/zephyr that referenced this pull request Aug 15, 2024
I broke the rendering of the CFB_FONT_MSB_FIRST font in
the previously committed zephyrproject-rtos#53796.
I will correct the entire font rendering process to make it
easier to remove the restriction that the vertical size of
the font must be a multiple of 8.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Chenhongren pushed a commit to Chenhongren/zephyr that referenced this pull request Aug 26, 2024
I broke the rendering of the CFB_FONT_MSB_FIRST font in
the previously committed zephyrproject-rtos#53796.
I will correct the entire font rendering process to make it
easier to remove the restriction that the vertical size of
the font must be a multiple of 8.

(cherry picked from commit d542e75)

Original-Signed-off-by: TOKITA Hiroshi <[email protected]>
GitOrigin-RevId: d542e75
Change-Id: Ie6c19b7fd8349c67cb116af7abbde49f4a78997f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5669144
Reviewed-by: Fabio Baltieri <[email protected]>
Commit-Queue: Fabio Baltieri <[email protected]>
Reviewed-by: Eric Yilun Lin <[email protected]>
Tested-by: ChromeOS Prod (Robot) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants