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

i#2283: Adds more scales to Umbra #2309

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

Conversation

johnfxgalea
Copy link
Contributor

@johnfxgalea johnfxgalea commented Aug 31, 2020

Adds more scales to Umbra. In particular, the following scales are added:

  • UMBRA_MAP_SCALE_DOWN_64X
  • UMBRA_MAP_SCALE_DOWN_32X
  • UMBRA_MAP_SCALE_UP_16X
  • UMBRA_MAP_SCALE_UP_4X
  • UMBRA_MAP_SCALE_UP_8X

Adds support for both 32-bit and 64-bit architectures.

Issue: #2283

@johnfxgalea johnfxgalea marked this pull request as draft August 31, 2020 16:33
@johnfxgalea
Copy link
Contributor Author

Blocked by #2310

@johnfxgalea johnfxgalea marked this pull request as ready for review September 1, 2020 22:59
@johnfxgalea
Copy link
Contributor Author

Blocked by #2310

Added the fixes to this PR after all.

@johnfxgalea
Copy link
Contributor Author

@derekbruening Not sure if you are aware that this PR is ready for review. No rush though as I am busy with work, so feel free to look at it at your convenience.

Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

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

Are there any more tests or sample tools to add that use these new features?

umbra/umbra.h Outdated Show resolved Hide resolved
umbra/umbra.h Outdated Show resolved Hide resolved
UMBRA_MAP_SCALE_UP_2X, /** 1 app byte to 2 shadow byte */
UMBRA_MAP_SCALE_UP_4X, /**
* 1 app byte to 4 shadow bytes.
* Reserve regions not supported (applies only for 64-bit).
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the user know what a "Reserve region" is? Not seeing it anywhere...ok it's right above in the new comments. Maybe point there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The doc you mentioned pertains to the same enum def. If I place a doxygen link, it's just going to take the reader to the enum.

umbra/umbra_64.c Outdated Show resolved Hide resolved
umbra/umbra_64.c Outdated Show resolved Hide resolved
umbra/umbra_64.c Outdated Show resolved Hide resolved
umbra/umbra_64.c Outdated Show resolved Hide resolved
umbra/umbra_64.c Outdated Show resolved Hide resolved
umbra/umbra_64.c Outdated Show resolved Hide resolved
umbra/umbra_64.c Outdated Show resolved Hide resolved
@johnfxgalea
Copy link
Contributor Author

johnfxgalea commented Sep 8, 2020

Many thanks for the review.

I have tested many of the features in my tools' code-bases. However, I'll write some quick sample tools and include them in this PR.

@johnfxgalea johnfxgalea changed the title i2283: Adds more scales to Umbra i#2283: Adds more scales to Umbra Jan 29, 2021
@johnfxgalea johnfxgalea marked this pull request as draft January 31, 2021 02:38
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.

None yet

2 participants