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

Clarify application of padding (#221) #233

Merged
merged 8 commits into from
May 3, 2017

Conversation

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

Looks good to me, one minor language change suggestion.

spec/ttml1.xml Outdated
@@ -5045,6 +5045,17 @@ then a presentation processor must use the closest supported value.</p>
the computed padding and the supported padding is minimized. If there are multiple closest supported values equally distant from
the computed value, then the value most distant from 0, i.e., the greatest padding, is used.</p>
</note>
<note role="elaboration">
<p>Padding space is applied as inset space to the part of the region's area that corresponds to the large allocation rectangle
in <bibref ref="xsl11"/>. Because the extent of a TTML region defines the fixed dimensions of the large allocation rectangle,
Copy link
Contributor

Choose a reason for hiding this comment

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

This may be clearer to some readers if we change "Because" to "Since".

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review and the correction!

@nigelmegitt
Copy link
Contributor

[Meeting 2017-03-09] agreed to apply the change requested by Nigel, then create as an errata and log the change.

spec/ttml1.xml Outdated
To get the top left vertex of the content rectangle the values for padding-start and padding-before need to be added the region's
origin (subject to the value of the region's writingMode).
</p>
</note>
Copy link
Contributor

Choose a reason for hiding this comment

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

This note is overly detailed and long, and introduces conceptual dependencies on XSL-FO which we have generally avoided elsewhere (except for section 9.3.3). I am replacing this with two notes in this section (8.2.16), one that reiterates and emphasizes the previously supplied parenthetical "(or inset)", and another just after the example that elaborates the details of insetting while explicitly describing the extent of the remaining content rectangle. In addition, I am adding a note in section (8.2.7), the definition of tts:extent, that additionally notes that padding is interior to the region's extent.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the key is to carefully define what region area means since there is no defined XSL equivalent. For instance, tts:overflow attribute defines whether a region area is clipped or not if the descendant areas of the region overflow its extent, but I assume that the clipping occurs outside the inset area, i.e. the area resulting from the region inset by padding, and not outside the region area.

spec/ttml1.xml Outdated
attributes that express a rectangle equivalent to the region's origin and
extent, and with a <att>line-stacking-strategy</att> attribute with value <code>line-height</code>;</p>
attributes that express a rectangle equivalent to the region's origin,
extent and padding, and with a <att>line-stacking-strategy</att> attribute with value <code>line-height</code>;</p>
</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing "and padding" to "(including padding)".

@andreastai
Copy link
Author

Thanks @skynavga for proposing some further changes to the PR. I need some time to review the differences.

@skynavga skynavga closed this Apr 24, 2017
@nigelmegitt
Copy link
Contributor

@skynavga I'm puzzled - did you mean to close this pull request without merging it after editing the branch?

@skynavga
Copy link
Contributor

skynavga commented Apr 24, 2017 via email

@nigelmegitt
Copy link
Contributor

There's a better way - the PR can be edited to be based on a different source/destination branch rather than creating a new PR.

@nigelmegitt nigelmegitt changed the base branch from master to gh-pages April 24, 2017 15:03
@nigelmegitt
Copy link
Contributor

I've changed the base branch as mentioned.

@nigelmegitt nigelmegitt reopened this Apr 24, 2017
@skynavga
Copy link
Contributor

ok, but next time, let me make the change so I will know how to do so in the future

@nigelmegitt
Copy link
Contributor

OK. For future reference, this was done by the following steps:

  1. Click the Edit button to the right of the pull request title at the top of this page.
  2. Change the base branch using the pull-down menu item.
  3. Click Save.
  4. if you accept the warning that some commits may be overwritten, rather than cancelling, the change occurs.

@andreastai
Copy link
Author

Thanks @nigelmegitt for your effort to keeping the info in the existing pull request! That makes tracking of the progress much easier.

@skynavga
Copy link
Contributor

Can we merge this yet? Any further comments?

@andreastai
Copy link
Author

No. As commented before I need to review the change to the PR.

@skynavga
Copy link
Contributor

skynavga commented Apr 25, 2017 via email

@andreastai
Copy link
Author

Yes, starting from 2017-04-20.

@skynavga
Copy link
Contributor

skynavga commented Apr 25, 2017 via email

@andreastai
Copy link
Author

andreastai commented Apr 25, 2017

The edit made by @skynavga is actually a new edit and it shortens the semantics of the original PR considerably.

The intent of the original PR was to clarify the relationship to XSL-FO and how TTML (in this case) "diverge" from the FO layout model. It was made more detailed here, because the implementer needs to understand fully what the difference is.

The new edit leaves out this content (by intent). To say that padding is an inset space of the region area does not help because it is never said "what" area of the region is meant. The addition that the extent of the region includes the padding may be sufficient for some readers, but I prefer as stated a more detailed explanation.

This should be reviewed also by others to decide which edit should be merged.

@skynavga
Copy link
Contributor

skynavga commented Apr 25, 2017 via email

@nigelmegitt
Copy link
Contributor

Let me remind you. Editing the TTML specifications is not a public or group
activity. It is an editor's activity. The group members and others serve
merely as contributing authors when submitting a PR. There should be no
expectation that the exact text of a PR will be accepted by the editor.

It is a Group decision what to publish as a Working Draft (or later transition state document). If the Editor has taken a view for which there is no consensus in the Group then an edit will usually be needed to resolve that lack of consensus. So if the Editor chooses not to take Group input on-board then there should be no expectation that the Editor's Draft will be accepted by the Group and published. This tension is best avoided by working together to generate changes that everyone can live with.

Here we have two differing views of the problem and the appropriate solution, and indeed the seriousness of that problem. I suggest we discuss this in our weekly call tomorrow.

@skynavga
Copy link
Contributor

skynavga commented Apr 26, 2017 via email

@skynavga
Copy link
Contributor

skynavga commented Apr 27, 2017 via email

@skynavga
Copy link
Contributor

skynavga commented Apr 27, 2017 via email

@andreastai
Copy link
Author

TTML intentionally avoids going into details about possible
implementation approaches for a number of reasons: (a) it makes the
specification longer; (b) it can be or become overly prescriptive, and lead
to brittle implementations; (c) it can have negative affects on testing and
on acceptance criteria as a content specification;

TTML 1 styling and layout depends in most cases on XSL-FO as conceptual model. How padding is applied (or extent of the region is defined) diverge considerably from XSL-FO. This is the reason why more details have been given in the original PR. It is useful as background information to use XSL-FO as conceptual model despite this derivation.

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

This looks good to me in the sense that it clearly addresses the issue raised, and I doubt that anyone could remain in any doubt about the relationship between the region's extent, the padding and the content area.

I'm not sure it addresses use of the term "region area" but that should probably be a separate issue.

@palemieux
Copy link
Contributor

I'm not sure it addresses use of the term "region area" but that should probably be a separate issue.

Yes.

@skynavga
Copy link
Contributor

skynavga commented May 2, 2017

Note that the expression "region area" in TTML1 is not new, and is essentially a reference to the definition of "Region" found in the terminology section, i.e., "a rectangular area of a presentation surface". Adding a simple ", i.e., a region area," parenthetical in that term definition would suffice.

@skynavga skynavga merged commit 9b65997 into gh-pages May 3, 2017
@skynavga skynavga deleted the issue-0221-clarify-padded-rectangle branch March 11, 2018 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants