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

Expand video memory slot to 32bit for unicode, and support multi-codepage. #1249

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@cloudwu
Contributor

cloudwu commented Oct 2, 2017

This patch expand video memory slots form uint_8[] to struct MemSlot . Each slot is 32 bit now, 24 bit for code point and 8 bit for attribute.

bgfx::dbgTextPrintf now support utf-8 string, it tries to convert unicode to cp437 ( https://en.wikipedia.org/wiki/Code_page_437 ) first , if done, stores the code point into low 8 bit of memory slot, otherwise stores unicode+256 .

I also support cp936 (simplify Chinese / GBK) subset of unicode. We can easily add more code page , or disable it.

In function void TextVideoMemBlitter::init() , this line enable cp936 support. Or we can add other code page here.

	// comment out this line to disable simplified Chinese (CP936) supported.
	TextCodePage::enable_cp936(this);

NOTICE: Some unicode (code point > 127) character's width is the same with ascii/cp437. We need modify this function: https://github.com/cloudwu/bgfx/blob/unicode/src/codepage.h#L21-L24

static int doublewidth(int unicode) {
	// todo: support multi-codepage
	return unicode > 127;
}

TextVideoMemBlitter know the width of glyph, it doesn't need it, but TextVideoMem::printfVargs call this function to decide whether insert a empty space following.

@cloudwu cloudwu changed the title from Expand video memory slot to 32bit for unicode. to Expand video memory slot to 32bit for unicode, and support multi-codepage. Oct 2, 2017

@bkaradzic

This comment has been minimized.

Show comment
Hide comment
@bkaradzic

bkaradzic Feb 14, 2018

Owner

This change is too complex for what I expected. I want to support this, but not at this level of complexity. I'll think how to make it work and be simple.

Owner

bkaradzic commented Feb 14, 2018

This change is too complex for what I expected. I want to support this, but not at this level of complexity. I'll think how to make it work and be simple.

@bkaradzic bkaradzic closed this Feb 14, 2018

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu Feb 17, 2018

Contributor

Could you merge this first?

118316f

This expand video memory slot to 32bit .

Contributor

cloudwu commented Feb 17, 2018

Could you merge this first?

118316f

This expand video memory slot to 32bit .

@bkaradzic

This comment has been minimized.

Show comment
Hide comment
@bkaradzic

bkaradzic Feb 17, 2018

Owner

Does it work correctly with 00-helloworld example?

Owner

bkaradzic commented Feb 17, 2018

Does it work correctly with 00-helloworld example?

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu Feb 18, 2018

Contributor

Of course. It doesn’t change the API. It only expand internal data structure (for future unicode support).

Contributor

cloudwu commented Feb 18, 2018

Of course. It doesn’t change the API. It only expand internal data structure (for future unicode support).

@bkaradzic

This comment has been minimized.

Show comment
Hide comment
@bkaradzic

bkaradzic Feb 18, 2018

Owner

Ok change code to match the style:

And make MemSlot:

struct MemSlot
{
    uint8_t attribute;
    uint8_t character;
};

So that by default doesn't change anything, but be more configurable for future use (and your needs).

Then submit PR.

Owner

bkaradzic commented Feb 18, 2018

Ok change code to match the style:

And make MemSlot:

struct MemSlot
{
    uint8_t attribute;
    uint8_t character;
};

So that by default doesn't change anything, but be more configurable for future use (and your needs).

Then submit PR.

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu Feb 19, 2018

Contributor

OK. I’m on vacation these days, I’ll submit PR later.

Contributor

cloudwu commented Feb 19, 2018

OK. I’m on vacation these days, I’ll submit PR later.

cloudwu added a commit to cloudwu/bgfx that referenced this pull request Feb 21, 2018

bkaradzic added a commit that referenced this pull request Feb 21, 2018

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu Feb 22, 2018

Contributor

@bkaradzic I think we can support utf-8 and multi codepage step by step. So how about add utf-8 support first (only support cp437) ?

Full unicode set has more than 100K code points , so we can't put them all into one textures, and manage multiple textures is too complex. All we needs is support few (user configurable) code pages.

I agree this PR is a little complex . I can rewrite a new version only support utf-8 string and one code page (cp437) first, and do not change the MemSlot structure.

Contributor

cloudwu commented Feb 22, 2018

@bkaradzic I think we can support utf-8 and multi codepage step by step. So how about add utf-8 support first (only support cp437) ?

Full unicode set has more than 100K code points , so we can't put them all into one textures, and manage multiple textures is too complex. All we needs is support few (user configurable) code pages.

I agree this PR is a little complex . I can rewrite a new version only support utf-8 string and one code page (cp437) first, and do not change the MemSlot structure.

@bkaradzic

This comment has been minimized.

Show comment
Hide comment
@bkaradzic

bkaradzic Feb 22, 2018

Owner

I need to add utf8 support first to bx.

Owner

bkaradzic commented Feb 22, 2018

I need to add utf8 support first to bx.

pigpigyyy added a commit to pigpigyyy/bgfx that referenced this pull request Mar 27, 2018

Merge branch 'master' into stencilMask
* master: (31 commits)
  Cleanup.
  Cleanup.
  Add struct MemSlot for future use, See #1249 (#1342)
  OSX: Disabled glsl-optimizer warnings.
  OSX: Fixed warnings.
  Prevent invalid texture update calls to immutable texture. Issue #1338.
  Updated ImGui.
  Updated Vulkan header.
  GLES3: Fixed build.
  Reverted last commit.
  GLES3: Fixed build.
  Removed use of wchar_t.
  GL: Fixed runtime patching when interpolation qualifier is used.
  Fixed GLSL intepolator qualifier.
  Cleanup.
  Updated glslang.
  Updated ImGui.
  add missing return statement (#1335)
  Cleanup.
  Cleanup.
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment