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 custom settings #191

Closed
lc-soft opened this issue Dec 23, 2019 · 10 comments · Fixed by #211 or #212
Closed

Add custom settings #191

lc-soft opened this issue Dec 23, 2019 · 10 comments · Fixed by #211 or #212
Labels
🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt low priority

Comments

@lc-soft
Copy link
Owner

lc-soft commented Dec 23, 2019

Issuehunt badges

Describe the solution you'd like

  • Add LCUI_Settings settings to src/main.c:MainApp.
  • Add files:
    • src/setttings.c
    • include/LCUI/settings.h
  • Change related files to apply settings:
    • src/main.c
    • src/gui/widget_task.c
    • src/display.c
    • ...

Usage:

LCUI_SettingsRec settings;

// Get global settings of the LCUI
Settings_Init(&settings);

// default settings:
// settings.paint_flashing = FALSE
// settings.Frame_rate_cap = 120
// settings.fps_meter = FALSE
// settings.record_profile = FALSE
// settings.parallel_rendering_threads = 4

// Set frame rate cap to 30
settings.frame_rate_cap = 30;

// Set parallel rendering threads to 8
settings.parallel_rendering_threads = 8;

// enable paint flashing
settings.paint_flashing = TRUE;

// start record performance profile
settings.record_profile = TRUE;

// show fps meter
settings.fps_meter = TRUE;

// apply this new settings, and trigger LCUI_SETTINGS_CHANGE event
LCUI_ApplySettings(&settings);

// Reset settings to default
LCUI_ResetSettings();

Additional context

settings.frame_rate_cap:

Call StepTimer_SetFrameLimit(MainApp.settings.frame_rate_cap) when LCUI_SETTINGS_CHANGE event trigger

LCUI/src/main.c

Line 474 in 949db38

StepTimer_SetFrameLimit(MainApp.timer, LCUI_MAX_FRAMES_PER_SEC);

settings.paint_flashing:

  • Remove display.show_rect_border, LCUIDisplay_ShowRectBorder(), LCUIDisplay_HideRectBorder().
  • Add LCUI_Settings settings to the display struct, and replace display.show_rect_border with display.settings->paint_flashing

if (display.show_rect_border) {

settings.parallel_rendering_threads:

  • Use display.settings->parallel_rendering_threads instead of PARALLEL_RENDERING_THREADS
  • The minimum value of parallel_rendering_threads is 1

DirtyLayerRec layers[PARALLEL_RENDERING_THREADS];

settings.record_profile:

  • Save profile data to MainApp.profile
  • Add LCUI_GetProfile() to get performance profile

LCUI/src/main.c

Line 183 in 949db38

void LCUI_RunFrameWithProfile(LCUI_FrameProfile profile)

settings.show_fps_meter:

No related code needs to be modified


IssueHunt Summary

jduo jduo has been rewarded.

Backers (Total: $50.00)

Submitted pull Requests


Tips

@issuehunt-oss
Copy link

issuehunt-oss bot commented Dec 23, 2019

@lc-soft has funded $15.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Dec 23, 2019
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jan 4, 2020

@lc-soft has funded $15.00 to this issue.


@lc-soft
Copy link
Owner Author

lc-soft commented Feb 28, 2020

This issue does not need to be processed, its content needs to be updated

@lc-soft lc-soft removed easy This issue is easy to solve help wanted labels Mar 10, 2020
@lc-soft lc-soft unpinned this issue Mar 28, 2020
@jduo
Copy link
Contributor

jduo commented Jun 24, 2020

@lc-soft , I've started this issue.

  • It looks as though the changes related to paint_flashing aren't needed. The fields and methods referenced are gone.
  • Do you have suggestions on how to validate this?
  • Is the funding on it still valid?

@jduo jduo mentioned this issue Jun 24, 2020
19 tasks
@lc-soft
Copy link
Owner Author

lc-soft commented Jun 24, 2020

@lc-soft , I've started this issue.

Please pause first.

It looks as though the changes related to paint_flashing aren't needed. The fields and methods referenced are gone.

LCUI/src/display.c

Lines 85 to 93 in 17d7d86

static struct LCUI_DisplayModule {
unsigned width, height;
LCUI_BOOL active;
LCUI_BOOL enable_paint_flashing;
LCUI_DisplayMode mode;
LinkedList surfaces;
LinkedList rects;
LCUI_DisplayDriver driver;
} display;

Do you have suggestions on how to validate this?

Please give me time to consider.

Is the funding on it still valid?

The current bonus is $30, but I see that you are requesting an increase of $50. It has exceeded my budget,I need to consider it because the priority of this issue is very low.

image

@jduo
Copy link
Contributor

jduo commented Jun 24, 2020

@lc-soft , I've started this issue.

Please pause first.

It looks as though the changes related to paint_flashing aren't needed. The fields and methods referenced are gone.

LCUI/src/display.c

Lines 85 to 93 in 17d7d86

static struct LCUI_DisplayModule {
unsigned width, height;
LCUI_BOOL active;
LCUI_BOOL enable_paint_flashing;
LCUI_DisplayMode mode;
LinkedList surfaces;
LinkedList rects;
LCUI_DisplayDriver driver;
} display;

Thanks. I'll update this struct. I believe the changelog mentions that LCUIDisplay_ShowRectBorder() LCUIDisplay_HideRectBorder() have been removed though. Is there anything I need to do related to these calls?

Do you have suggestions on how to validate this?

Please give me time to consider.

Is the funding on it still valid?

The current bonus is $30, but I see that you are requesting an increase of $50. It has exceeded my budget,I need to consider it because the priority of this issue is very low.

I really meant that as an increase to $50, not an increase of $50. I'm new to IssueHunt and may have issued the request incorrectly. The $20 increase was largely about my uncertainty with working with event handlers in this project and being a bit unclear on how to verify this.

image

@issuehunt-oss
Copy link

issuehunt-oss bot commented Jun 25, 2020

@lc-soft has funded $20.00 to this issue.


@lc-soft
Copy link
Owner Author

lc-soft commented Jun 25, 2020

@jduo

I really meant that as an increase to $50, not an increase of $50. ...

The bonus has been increased to $50, please note that issuehunt has a service fee (10%).

Is there anything I need to do related to these calls?

LCUI/src/display.c

Lines 624 to 629 in 17d7d86

void LCUIDisplay_EnablePaintFlashing(LCUI_BOOL enable)
{
display.enable_paint_flashing = enable;
}

Do you have suggestions on how to validate this?

how to validate this? Do you mean how to validate that the code in this issue is working properly? Well, you need to add test/test_settings.c to test whether these settings take effect.

Other suggestions:

  • settings.record_profile: Use Settings check code instead of #ifdef DEBUG

    LCUI/src/main.c

    Lines 410 to 424 in 949db38

    DEBUG_MSG("loop: %p, enter\n", loop);
    MainApp.loop = loop;
    #ifdef DEBUG
    LCUIProfile_Init(&profile);
    #endif
    while (loop->state != STATE_EXITED) {
    #ifdef DEBUG
    frame = LCUIProfile_BeginFrame(&profile);
    LCUI_RunFrameWithProfile(frame);
    LCUIProfile_EndFrame(&profile);
    #else
    LCUI_RunFrame();
    #endif
    StepTimer_Remain(MainApp.timer);
    /* 如果当前运行的主循环不是自己 */
  • My design is simple, and it may have problems. If you know a better design pattern, or some best practices for similar problems, you can try it.

jduo added a commit to jduo/LCUI that referenced this issue Jun 25, 2020
- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes

Fixes lc-soft#192
@jduo
Copy link
Contributor

jduo commented Jun 25, 2020

I've put up an initial PR with what I think cover the code changes required.
Note that in display.c, you've suggested holding a pointer to the settings, but the API only lets you copy the global settings (which I feel is safer than giving direct access to the global settings, especially with regards to sending event settings change notifications).

I'll work on adding unit tests next.

jduo added a commit to jduo/LCUI that referenced this issue Jun 30, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jun 30, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jun 30, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jun 30, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jun 30, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 1, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
lc-soft pushed a commit that referenced this issue Jul 2, 2020
BREAKING CHANGE: LCUIDisplay_EnablePaintFlashing() has been removed.

- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes #192

Co-authored-by: James Duong <jduong@dremio.com>
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
Restore and re-implement LCUIDisplay_EnablePaintFlashing.

Fixes lc-soft#192
jduo added a commit to jduo/LCUI that referenced this issue Jul 2, 2020
Restore and re-implement LCUIDisplay_EnablePaintFlashing.

Fixes lc-soft#191
lc-soft pushed a commit that referenced this issue Jul 2, 2020
Restore and re-implement LCUIDisplay_EnablePaintFlashing.

Fixes #191
lc-soft pushed a commit that referenced this issue Jul 2, 2020
- Add API for retrieving and modifying global settings
- Utilize this API in the codebase
- Add event for when global settings are changed
- Add event handler for updating the frame rate cap
- Add event handler to notify paint flashing changes
- Add unit tests

Fixes #192

Co-authored-by: James Duong <jduong@dremio.com>

feat: add settings api (#191) (#211) (#212)

Restore and re-implement LCUIDisplay_EnablePaintFlashing.

Fixes #191
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jul 2, 2020

@lc-soft has rewarded $45.00 to @jduo. See it on IssueHunt

  • 💰 Total deposit: $50.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $5.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt low priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants