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

Layout viewer #76

Closed
mominul opened this issue Oct 14, 2018 · 12 comments
Closed

Layout viewer #76

mominul opened this issue Oct 14, 2018 · 12 comments
Assignees

Comments

@mominul
Copy link
Member

mominul commented Oct 14, 2018

Layout viewer resizes every time whenever the layout is changed. This bug has appeared after the layout management changes in Layout Viewer UI.

layout viewer_bug

@mominul mominul added the Bug label Oct 14, 2018
@mominul
Copy link
Member Author

mominul commented Oct 14, 2018

CC @Adyel

@Adyel
Copy link
Member

Adyel commented Oct 17, 2018

@mominul Where are those image (Layout image) located? I could not find them.

@mominul
Copy link
Member Author

mominul commented Oct 17, 2018

Layout images are Base64 encoded string located in the layout(json) files. Those are in image0 and image1 keys, like in Bornona layout. Images are loaded by this code: https://github.com/OpenBangla/OpenBangla-Keyboard/blob/master/src/frontend/LayoutViewer.cpp#L61-L82

@Adyel
Copy link
Member

Adyel commented Oct 17, 2018

Oh, wow. Why are we using base64 on a desktop application? 😲 We should update it soon. There is an embedded Avro logo and the image is also low res. This will be a lengthy work but we should do it. We can also implement zoom functionality after that.
Opinion?

Tools to use: http://www.keyboard-layout-editor.com/#/

NB: I set the size constraint for the image. The window does not change size anymore but you can still tell the diffrerence because the base image is different

Monir Optima, Provat use one keyboard image
Avro Easy, Bornona, Jatiya use different one.

@Adyel
Copy link
Member

Adyel commented Oct 17, 2018

It was happening because the window size was hardcoded on the LayoutViewer.cpp. It was overriding the UI configuration.

this->setFixedHeight(ui->labelImage->height() + ui->labelImage->y());
this->setFixedWidth(ui->labelImage->width());

Adyel added a commit to Adyel/OpenBangla-Keyboard that referenced this issue Oct 17, 2018
@mominul
Copy link
Member Author

mominul commented Oct 17, 2018

Oh, wow. Why are we using base64 on a desktop application? 😲 We should update it soon. There is an embedded Avro logo and the image is also low res. This will be a lengthy work but we should do it. We can also implement zoom functionality after that.
Opinion?

👍 agreed! But can you elaborate on why using Base64 is bad? We are only using it to embed the images in the layout file. Are there any alternate method?

We should update it soon. There is an embedded Avro logo and the image is also low res. This will be a lengthy work but we should do it.

Are you saying to update the images here? I am confused 😕 Zooming functionality would be great!

Edit
http://www.keyboard-layout-editor.com/#/
Wow, I didn't know about this tool! We can easily make layout images!

@Adyel
Copy link
Member

Adyel commented Oct 17, 2018

Why I think we should change base64.

  • It was not designed for the Desktop application. It was originally used for the web. In some case, it can speed up loading time (reduce some web overhead).
  • It added reliability for web transfer. Back in the days, file corruption was a big pain and it was used to deal with that. We don't need that here.
  • It is very low resolution and hard to manipulate.
  • It already contains an embedded logo which does not look good.
  • base64 is not an efficient storing method. It uses more space than the original binary. (~130%)
  • We can use SVG and scale the image as much as we want.

We need to change the way we import layout. We can implement this feature not compromising backward compatibility. Our Layout package will be a compressed .zip file which will include the layout JSON and an SVG image for LayoutViewer.

@mominul
Copy link
Member Author

mominul commented Oct 17, 2018

👍 for SVG. SVG is a textual format, I think we can easily embed it into a JSON file. Zip approach is not that bad, but wouldn't it bring complexity?

@Adyel
Copy link
Member

Adyel commented Oct 17, 2018

I personally think zip is a better way of handling things. Think of it as a package rather than a file. The data will be also compressed.

@mominul
Copy link
Member Author

mominul commented Oct 17, 2018

@Adyel Can you make a prototype? Because I haven't yet written code handling Zip files.

@Adyel
Copy link
Member

Adyel commented Oct 17, 2018

@mominul I'll look into it. Meanwhile, why not merge the PR? At least the window size will be consistent or you are having a second thought. 🤔

@mominul
Copy link
Member Author

mominul commented Oct 17, 2018

I think the images seem really odd if we fix the image's size. You can compare the results.

@Adyel Adyel added the Stale label May 1, 2019
@Adyel Adyel closed this as completed May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants