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

Refactor rendering to use CSwordKey as a key to render #363

Open
wants to merge 5 commits into
base: wip/kalemas/fix-verse-range-rendering-in-info
Choose a base branch
from

Conversation

kalemas
Copy link
Member

@kalemas kalemas commented Jan 8, 2022

This PR resolve issue when parallel view didn't respected mapping between versifications.
bibletime_DNMynNOwQ4

I expect it will also bring some architecture optimizations. Now it is just a rough idea of change so many things have to be reconsidered and deduplicated before merge.

Modern cpp moved forward so i m not sure it follows best practices, please correct me.

@kalemas kalemas self-assigned this Jan 8, 2022
@kalemas kalemas linked an issue Jan 8, 2022 that may be closed by this pull request
@jaakristioja
Copy link
Member

Since you want to merge it into wip/kalemas/fix-verse-range-rendering-in-info is suppose this depends on pull request #362?

@kalemas
Copy link
Member Author

kalemas commented Jan 9, 2022

Since you want to merge it into wip/kalemas/fix-verse-range-rendering-in-info is suppose this depends on pull request #362?

Yes, i m sure it require changes from that PR but for review i d prefer to not mix changes from another PR.

So i will proceed to develop this and will propose ready to merge variant next time.

@kalemas kalemas force-pushed the wip/kalemas/fix-verse-range-rendering-in-info branch from ad67bc7 to c7ad0fe Compare January 12, 2022 21:20
@kalemas kalemas changed the base branch from wip/kalemas/fix-verse-range-rendering-in-info to stable-3.0 January 15, 2022 21:36
@kalemas kalemas force-pushed the wip/kalemas/refactor-rendering branch from fc888df to f7a3e06 Compare January 16, 2022 20:16
@kalemas kalemas changed the base branch from stable-3.0 to wip/kalemas/fix-verse-range-rendering-in-info January 16, 2022 20:16
@kalemas kalemas force-pushed the wip/kalemas/refactor-rendering branch 4 times, most recently from 94f58c2 to ee1e9dc Compare January 16, 2022 22:13
@kalemas kalemas force-pushed the wip/kalemas/refactor-rendering branch from ee1e9dc to 839b7c8 Compare January 16, 2022 22:31
@kalemas
Copy link
Member Author

kalemas commented Jan 16, 2022

@jaakristioja i think that this may be reviewed now. I would say fair, i see no problems with passing by pointer/references, constancy. I also split main commit in two that changing methods signatures.

@kalemas
Copy link
Member Author

kalemas commented Jan 16, 2022

it is now rendered correctly at my sight
image

@kalemas kalemas force-pushed the wip/kalemas/fix-verse-range-rendering-in-info branch from be2b6f4 to ece8bf2 Compare January 17, 2022 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix v11n mapping in parallel views
2 participants